Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-06-20 Thread Tomasz Janiszewski


> On Maj 11, 2017, 8:35 rano, Tomasz Janiszewski wrote:
> > src/webui/master/static/browse.html
> > Lines 17-20 (original), 17-20 (patched)
> > 
> >
> > How about usign `` here instead of list? Then `/` will be just 
> > visible text and then copyalbe whitouth hacks with zero sized text.
> > 
> > 
> > I haven't tested this but below code is what I think it could look like.
> > ```
> > 
> >   
> >  
> > {{dir}}
> >  
> >  /
> >   
> > 
> > ```
> 
> Tomasz Janiszewski wrote:
> I've tested following code. It looks similar
> ```diff
> 
> diff --git a/src/webui/master/static/browse.html 
> b/src/webui/master/static/browse.html
> index b984919..0594f81 100644
> --- a/src/webui/master/static/browse.html
> +++ b/src/webui/master/static/browse.html
> @@ -11,14 +11,15 @@
>
>  
> 
> -
> -  
> -
> +  
> + href="#/agents/{{agent_id}}/browse?path={{
>   encodeURIComponent(path.split('/').slice(0, $index + 
> 1).join('/'))}}">
> +  /
>{{dir}}
>  
> -  
> -
> +  
> +
> 
>  
>×
> diff --git a/src/webui/master/static/css/mesos.css 
> b/src/webui/master/static/css/mesos.css
> index 9f3de54..21e3ef1 100644
> --- a/src/webui/master/static/css/mesos.css
> +++ b/src/webui/master/static/css/mesos.css
> @@ -41,6 +41,11 @@
>content: "/";
>  }
> 
> +.dir {
> +  padding: 0 5px 0 0;
> +  color: #ccc;
> +}
> +
>  /*
>   * /BOOTSTRAP OVERRIDES
>   */
> 
> ```
> 
> haosdent huang wrote:
> Hi, @janisz very sorry for the delay. I try this patch, but the spcaces 
> is still there. Seems didn't resolve our problem.

No problem. I think we can merge it as is.


- Tomasz


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/#review174615
---


On Cze 20, 2017, 7:58 rano, haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated Cze 20, 2017, 7:58 rano)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Bugs: MESOS-7468
> https://issues.apache.org/jira/browse/MESOS-7468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-06-20 Thread haosdent huang

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/
---

(Updated June 20, 2017, 7:58 a.m.)


Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.


Bugs: MESOS-7468
https://issues.apache.org/jira/browse/MESOS-7468


Repository: mesos


Description
---

Stripped spaces between directory elements in WebUI.


Diffs
-

  src/webui/master/static/browse.html b9849197227b06df348789a49348e2b5d4cfd2ae 
  src/webui/master/static/css/mesos.css 
9f3de5427071fc61d3791c4bc2a660368c2cd3c2 


Diff: https://reviews.apache.org/r/58874/diff/4/


Testing
---


Thanks,

haosdent huang



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-06-19 Thread haosdent huang

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/
---

(Updated June 20, 2017, 3:48 a.m.)


Review request for .


Bugs: MESOS-7468
https://issues.apache.org/jira/browse/MESOS-7468


Repository: mesos


Description
---

Stripped spaces between directory elements in WebUI.


Diffs
-

  src/webui/master/static/browse.html b9849197227b06df348789a49348e2b5d4cfd2ae 
  src/webui/master/static/css/mesos.css 
9f3de5427071fc61d3791c4bc2a660368c2cd3c2 


Diff: https://reviews.apache.org/r/58874/diff/4/


Testing
---


Thanks,

haosdent huang



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-06-19 Thread haosdent huang


> On May 11, 2017, 8:35 a.m., Tomasz Janiszewski wrote:
> > src/webui/master/static/browse.html
> > Lines 17-20 (original), 17-20 (patched)
> > 
> >
> > How about usign `` here instead of list? Then `/` will be just 
> > visible text and then copyalbe whitouth hacks with zero sized text.
> > 
> > 
> > I haven't tested this but below code is what I think it could look like.
> > ```
> > 
> >   
> >  
> > {{dir}}
> >  
> >  /
> >   
> > 
> > ```
> 
> Tomasz Janiszewski wrote:
> I've tested following code. It looks similar
> ```diff
> 
> diff --git a/src/webui/master/static/browse.html 
> b/src/webui/master/static/browse.html
> index b984919..0594f81 100644
> --- a/src/webui/master/static/browse.html
> +++ b/src/webui/master/static/browse.html
> @@ -11,14 +11,15 @@
>
>  
> 
> -
> -  
> -
> +  
> + href="#/agents/{{agent_id}}/browse?path={{
>   encodeURIComponent(path.split('/').slice(0, $index + 
> 1).join('/'))}}">
> +  /
>{{dir}}
>  
> -  
> -
> +  
> +
> 
>  
>×
> diff --git a/src/webui/master/static/css/mesos.css 
> b/src/webui/master/static/css/mesos.css
> index 9f3de54..21e3ef1 100644
> --- a/src/webui/master/static/css/mesos.css
> +++ b/src/webui/master/static/css/mesos.css
> @@ -41,6 +41,11 @@
>content: "/";
>  }
> 
> +.dir {
> +  padding: 0 5px 0 0;
> +  color: #ccc;
> +}
> +
>  /*
>   * /BOOTSTRAP OVERRIDES
>   */
> 
> ```

Hi, @janisz very sorry for the delay. I try this patch, but the spcaces is 
still there. Seems didn't resolve our problem.


- haosdent


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/#review174615
---


On June 20, 2017, 3:41 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated June 20, 2017, 3:41 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Bugs: MESOS-7468
> https://issues.apache.org/jira/browse/MESOS-7468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/4/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-06-19 Thread haosdent huang

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/
---

(Updated June 20, 2017, 3:41 a.m.)


Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.


Changes
---

Fix @benm's comments.


Bugs: MESOS-7468
https://issues.apache.org/jira/browse/MESOS-7468


Repository: mesos


Description
---

Stripped spaces between directory elements in WebUI.


Diffs (updated)
-

  src/webui/master/static/browse.html b9849197227b06df348789a49348e2b5d4cfd2ae 
  src/webui/master/static/css/mesos.css 
9f3de5427071fc61d3791c4bc2a660368c2cd3c2 


Diff: https://reviews.apache.org/r/58874/diff/4/

Changes: https://reviews.apache.org/r/58874/diff/3-4/


Testing
---


File Attachments


strip_space.gif
  
https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif


Thanks,

haosdent huang



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-05-11 Thread Tomasz Janiszewski


> On Maj 11, 2017, 8:35 rano, Tomasz Janiszewski wrote:
> > src/webui/master/static/browse.html
> > Lines 17-20 (original), 17-20 (patched)
> > 
> >
> > How about usign `` here instead of list? Then `/` will be just 
> > visible text and then copyalbe whitouth hacks with zero sized text.
> > 
> > 
> > I haven't tested this but below code is what I think it could look like.
> > ```
> > 
> >   
> >  
> > {{dir}}
> >  
> >  /
> >   
> > 
> > ```

I've tested following code. It looks similar
```diff

diff --git a/src/webui/master/static/browse.html 
b/src/webui/master/static/browse.html
index b984919..0594f81 100644
--- a/src/webui/master/static/browse.html
+++ b/src/webui/master/static/browse.html
@@ -11,14 +11,15 @@
   
 

-
-  
-
+  
+
+  /
   {{dir}}
 
-  
-
+  
+

 
   ×
diff --git a/src/webui/master/static/css/mesos.css 
b/src/webui/master/static/css/mesos.css
index 9f3de54..21e3ef1 100644
--- a/src/webui/master/static/css/mesos.css
+++ b/src/webui/master/static/css/mesos.css
@@ -41,6 +41,11 @@
   content: "/";
 }

+.dir {
+  padding: 0 5px 0 0;
+  color: #ccc;
+}
+
 /*
  * /BOOTSTRAP OVERRIDES
  */

```


- Tomasz


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/#review174615
---


On Maj 8, 2017, 8:01 rano, haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated Maj 8, 2017, 8:01 rano)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Bugs: MESOS-7468
> https://issues.apache.org/jira/browse/MESOS-7468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/3/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-05-11 Thread Tomasz Janiszewski

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/#review174615
---




src/webui/master/static/browse.html
Lines 17-20 (original), 17-20 (patched)


How about usign `` here instead of list? Then `/` will be just 
visible text and then copyalbe whitouth hacks with zero sized text.

I haven't tested this but below code is what I think it could look like.
```

  
 
{{dir}}
 
 /
  

```


- Tomasz Janiszewski


On Maj 8, 2017, 8:01 rano, haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated Maj 8, 2017, 8:01 rano)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Bugs: MESOS-7468
> https://issues.apache.org/jira/browse/MESOS-7468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/3/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-05-10 Thread haosdent huang


> On May 11, 2017, 12:38 a.m., Benjamin Mahler wrote:
> > src/webui/master/static/browse.html
> > Lines 17-20 (original), 17-20 (patched)
> > 
> >
> > I suspect, if my understanding is correct, you could do the following 
> > format:
> > 
> > ```
> >   
> > {{dir}}
> > /
> >   
> > ```
> > 
> > Does that also insert a space?

Yes, this still insert a space

```
')">
https://reviews.apache.org/r/58874/#review174590
---


On May 8, 2017, 8:01 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated May 8, 2017, 8:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Bugs: MESOS-7468
> https://issues.apache.org/jira/browse/MESOS-7468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/3/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-05-10 Thread haosdent huang


> On May 11, 2017, 12:38 a.m., Benjamin Mahler wrote:
> > Ok, I understand now what's going on in this change. I gave some 
> > suggestions for comments / naming to clarify this.
> > 
> > It feels like a hack however, since I would expect the breadcrumb '/' 
> > characters to be getting copied. Do you understand why they're not getting 
> > copied? If not, please leave a TODO to look into that.

Thx @bmahler, 

> why they're not getting copied?

As I mentioned in https://issues.apache.org/jira/browse/MESOS-7468 , this is 
expected here because the definition of breadcrumb in bootstrap is

```
.breadcrumb > li + li:before {
content: "/";
}
```

`:before` selector make that content not getter copied here.


- haosdent


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/#review174590
---


On May 8, 2017, 8:01 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated May 8, 2017, 8:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Bugs: MESOS-7468
> https://issues.apache.org/jira/browse/MESOS-7468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/3/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-05-10 Thread Benjamin Mahler

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/#review174590
---


Fix it, then Ship it!




Ok, I understand now what's going on in this change. I gave some suggestions 
for comments / naming to clarify this.

It feels like a hack however, since I would expect the breadcrumb '/' 
characters to be getting copied. Do you understand why they're not getting 
copied? If not, please leave a TODO to look into that.


src/webui/master/static/browse.html
Lines 15-16 (original), 15-16 (patched)


This comment seems misleading, since we're not doing any stripping here, 
are we?

It seems that like Tomasz said, the spaces are being removed because now 
there are no spaces or newlines between the closing `` and `` tags?

How about this?

```

```

Is this accurate?



src/webui/master/static/browse.html
Lines 17-20 (original), 17-20 (patched)


I suspect, if my understanding is correct, you could do the following 
format:

```
  
{{dir}}
/
  
```

Does that also insert a space?



src/webui/master/static/css/mesos.css
Lines 179-181 (patched)


Ok I think I understand now, in that this is just making the text invisible 
using a 0 size font..?

If so, how about calling this `hidden-text`?

```
// The `hidden-text` class hides text by making the font size 0.
// This can be used, for example, to insert text that will be
// added when the user highlights and copies, while leaving
// the text hidden from the user.
```

This still feels like a hack to me, have you tried looking into whether we 
can just update the `breadcrumb` class to have the slashes show up when copying?


- Benjamin Mahler


On May 8, 2017, 8:01 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated May 8, 2017, 8:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Bugs: MESOS-7468
> https://issues.apache.org/jira/browse/MESOS-7468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/3/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-05-08 Thread Mesos Reviewbot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/#review174191
---



Patch looks great!

Reviews applied: [58874]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On May 8, 2017, 8:01 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated May 8, 2017, 8:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Bugs: MESOS-7468
> https://issues.apache.org/jira/browse/MESOS-7468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/3/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-05-08 Thread haosdent huang

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/
---

(Updated May 8, 2017, 8:01 a.m.)


Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.


Bugs: MESOS-7468
https://issues.apache.org/jira/browse/MESOS-7468


Repository: mesos


Description
---

Stripped spaces between directory elements in WebUI.


Diffs
-

  src/webui/master/static/browse.html b9849197227b06df348789a49348e2b5d4cfd2ae 
  src/webui/master/static/css/mesos.css 
9f3de5427071fc61d3791c4bc2a660368c2cd3c2 


Diff: https://reviews.apache.org/r/58874/diff/3/


Testing
---


File Attachments


strip_space.gif
  
https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif


Thanks,

haosdent huang



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-05-08 Thread haosdent huang

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/
---

(Updated May 8, 2017, 8 a.m.)


Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.


Changes
---

Fixed @bmahler's comments.


Repository: mesos


Description
---

Stripped spaces between directory elements in WebUI.


Diffs (updated)
-

  src/webui/master/static/browse.html b9849197227b06df348789a49348e2b5d4cfd2ae 
  src/webui/master/static/css/mesos.css 
9f3de5427071fc61d3791c4bc2a660368c2cd3c2 


Diff: https://reviews.apache.org/r/58874/diff/3/

Changes: https://reviews.apache.org/r/58874/diff/2-3/


Testing
---


File Attachments


strip_space.gif
  
https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif


Thanks,

haosdent huang



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-05-08 Thread haosdent huang


> On May 3, 2017, 7:45 p.m., Benjamin Mahler wrote:
> > src/webui/master/static/browse.html
> > Lines 15-16 (original), 15-16 (patched)
> > 
> >
> > Looking at the gif, it seems the slashes weren't copied before? Do you 
> > want to clarify that here? Specifically, it seems this also ensures the 
> > slashes are being copied?

> the slashes weren't copied before? Do you want to clarify that here?

Thx @bmahler, I create a ticket at 
https://issues.apache.org/jira/browse/MESOS-7468 to clarify the problem.

> Specifically, it seems this also ensures the slashes are being copied?

The strange line break here only ensure there are no spaces, so I didn't 
mention `/` at here.


- haosdent


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/#review173801
---


On May 3, 2017, 4:29 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated May 3, 2017, 4:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/2/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-05-03 Thread Benjamin Mahler

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/#review173801
---




src/webui/master/static/browse.html
Lines 15-16 (original), 15-16 (patched)


Looking at the gif, it seems the slashes weren't copied before? Do you want 
to clarify that here? Specifically, it seems this also ensures the slashes are 
being copied?



src/webui/master/static/css/mesos.css
Lines 179-181 (patched)


Can you add a comment on what this does and when one would need to use it?


- Benjamin Mahler


On May 3, 2017, 4:29 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated May 3, 2017, 4:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/2/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-05-03 Thread Mesos Reviewbot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/#review173796
---



Patch looks great!

Reviews applied: [58874]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On May 3, 2017, 4:29 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated May 3, 2017, 4:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/2/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-05-03 Thread Tomasz Janiszewski


> On Maj 1, 2017, 8:14 po południu, Tomasz Janiszewski wrote:
> > src/webui/master/static/browse.html
> > Line 17 (original), 17 (patched)
> > 
> >
> > Could you keep original formatting?
> 
> haosdent huang wrote:
> Change to
> 
> ```
>  href="#/agents/{{agent_id}}/browse?path={{
> encodeURIComponent(path.split('/').slice(0, $index + 
> 1).join('/'))
> }}">{{dir}}/
> ```
> 
> Is it match you expection here?

That's OK. We must keep everything in one line to prevent newlines and spaces 
to appear.


> On Maj 1, 2017, 8:14 po południu, Tomasz Janiszewski wrote:
> > src/webui/master/static/css/mesos.css
> > Lines 180 (patched)
> > 
> >
> > I think this should be replaced with `visibility: hidden;`. With 
> > `font-size: 0;` there is different (smaller) padding then before the change.
> 
> haosdent huang wrote:
> Oh, I didn't use it because with `visibility: hidden`, the copy content 
> would not include `` in my browser. Is it work in your side?

My mistake, I'm sorry.


- Tomasz


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/#review173499
---


On Maj 3, 2017, 4:29 rano, haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated Maj 3, 2017, 4:29 rano)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/2/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-05-02 Thread haosdent huang


> On May 1, 2017, 10:49 p.m., Benjamin Mahler wrote:
> > I'm a bit lost, is there a visual change here? If so, can you include the 
> > screenshots?
> > 
> > Could you spell out the problem? Is it that when trying to copy a path in 
> > the file browser, it includes unwanted spaces?

Oh, yes, it is confuse without any screenshot. I update a screenshot at 
https://reviews.apache.org/r/58874/file/1428/


- haosdent


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/#review173526
---


On May 3, 2017, 4:29 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated May 3, 2017, 4:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/2/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-05-02 Thread haosdent huang

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/
---

(Updated May 3, 2017, 4:29 a.m.)


Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.


Repository: mesos


Description
---

Stripped spaces between directory elements in WebUI.


Diffs
-

  src/webui/master/static/browse.html b9849197227b06df348789a49348e2b5d4cfd2ae 
  src/webui/master/static/css/mesos.css 
9f3de5427071fc61d3791c4bc2a660368c2cd3c2 


Diff: https://reviews.apache.org/r/58874/diff/2/


Testing
---


File Attachments (updated)


strip_space.gif
  
https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif


Thanks,

haosdent huang



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-05-02 Thread haosdent huang

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/
---

(Updated May 3, 2017, 4:25 a.m.)


Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.


Repository: mesos


Description
---

Stripped spaces between directory elements in WebUI.


Diffs (updated)
-

  src/webui/master/static/browse.html b9849197227b06df348789a49348e2b5d4cfd2ae 
  src/webui/master/static/css/mesos.css 
9f3de5427071fc61d3791c4bc2a660368c2cd3c2 


Diff: https://reviews.apache.org/r/58874/diff/2/

Changes: https://reviews.apache.org/r/58874/diff/1-2/


Testing
---


Thanks,

haosdent huang



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-05-02 Thread haosdent huang


> On May 1, 2017, 8:14 p.m., Tomasz Janiszewski wrote:
> > src/webui/master/static/browse.html
> > Line 17 (original), 17 (patched)
> > 
> >
> > Could you keep original formatting?

Change to

```
  {{dir}}/
```

Is it match you expection here?


> On May 1, 2017, 8:14 p.m., Tomasz Janiszewski wrote:
> > src/webui/master/static/css/mesos.css
> > Lines 180 (patched)
> > 
> >
> > I think this should be replaced with `visibility: hidden;`. With 
> > `font-size: 0;` there is different (smaller) padding then before the change.

Oh, I didn't use it because with `visibility: hidden`, the copy content would 
not include `` in my browser. Is it work in your side?


- haosdent


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/#review173499
---


On April 30, 2017, 4:29 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated April 30, 2017, 4:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-05-01 Thread Benjamin Mahler

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/#review173526
---



I'm a bit lost, is there a visual change here? If so, can you include the 
screenshots?

Could you spell out the problem? Is it that when trying to copy a path in the 
file browser, it includes unwanted spaces?

- Benjamin Mahler


On April 30, 2017, 4:29 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated April 30, 2017, 4:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-05-01 Thread Tomasz Janiszewski

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/#review173499
---


Fix it, then Ship it!




Nice usability improvement.


src/webui/master/static/browse.html
Line 17 (original), 17 (patched)


Could you keep original formatting?



src/webui/master/static/css/mesos.css
Lines 180 (patched)


I think this should be replaced with `visibility: hidden;`. With 
`font-size: 0;` there is different (smaller) padding then before the change.


- Tomasz Janiszewski


On Kwi 30, 2017, 4:29 rano, haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated Kwi 30, 2017, 4:29 rano)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-04-30 Thread Mesos Reviewbot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/#review173451
---



Patch looks great!

Reviews applied: [58874]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On April 30, 2017, 4:29 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated April 30, 2017, 4:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Review Request 58874: Stripped spaces between directory elements in WebUI.

2017-04-29 Thread haosdent huang

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58874/
---

Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.


Repository: mesos


Description
---

Stripped spaces between directory elements in WebUI.


Diffs
-

  src/webui/master/static/browse.html b9849197227b06df348789a49348e2b5d4cfd2ae 
  src/webui/master/static/css/mesos.css 
9f3de5427071fc61d3791c4bc2a660368c2cd3c2 


Diff: https://reviews.apache.org/r/58874/diff/1/


Testing
---


Thanks,

haosdent huang