You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by haosdent huang <ha...@apache.org> on 2017/04/30 04:29:37 UTC
Review Request 58874: Stripped spaces between directory elements in
WebUI.
-----------------------------------------------------------
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
Re: Review Request 58874: Stripped spaces between directory elements
in WebUI.
Posted by haosdent huang <ha...@apache.org>.
> 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.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
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.
Posted by Tomasz Janiszewski <ja...@gmail.com>.
> On Maj 11, 2017, 8:35 rano, Tomasz Janiszewski wrote:
> > src/webui/master/static/browse.html
> > Lines 17-20 (original), 17-20 (patched)
> > <https://reviews.apache.org/r/58874/diff/3/?file=1710161#file1710161line17>
> >
> > How about usign `<span>` 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.
> > ```
> > <div class="breadcrunb">
> > <span ng-repeat="dir in path.split('/')">
> > <a href="#/agents/{{agent_id}}/browse?path={{
> > encodeURIComponent(path.split('/').slice(0, $index + 1).join('/'))
> > }}">
> > {{dir}}
> > </a>
> > <span>/</span>
> > </span>
> > </div>
> > ```
>
> 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 @@
> </li>
> </ol>
>
> -<ol class="breadcrumb">
> - <li ng-repeat="dir in path.split('/')">
> - <a href="#/agents/{{agent_id}}/browse?path={{
> +<div class="breadcrumb">
> + <span ng-repeat="dir in path.split('/').slice(1)">
> + <a style="white-space: nowrap" href="#/agents/{{agent_id}}/browse?path={{
> encodeURIComponent(path.split('/').slice(0, $index + 1).join('/'))}}">
> + <span class="dir">/</span>
> {{dir}}
> </a>
> - </li>
> -</ol>
> + </span>
> +</div>
>
> <div class="alert alert-error hidden" id="alert">
> <button class="close" data-dismiss="alert">×</button>
> 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.
Posted by haosdent huang <ha...@apache.org>.
> On May 11, 2017, 8:35 a.m., Tomasz Janiszewski wrote:
> > src/webui/master/static/browse.html
> > Lines 17-20 (original), 17-20 (patched)
> > <https://reviews.apache.org/r/58874/diff/3/?file=1710161#file1710161line17>
> >
> > How about usign `<span>` 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.
> > ```
> > <div class="breadcrunb">
> > <span ng-repeat="dir in path.split('/')">
> > <a href="#/agents/{{agent_id}}/browse?path={{
> > encodeURIComponent(path.split('/').slice(0, $index + 1).join('/'))
> > }}">
> > {{dir}}
> > </a>
> > <span>/</span>
> > </span>
> > </div>
> > ```
>
> 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 @@
> </li>
> </ol>
>
> -<ol class="breadcrumb">
> - <li ng-repeat="dir in path.split('/')">
> - <a href="#/agents/{{agent_id}}/browse?path={{
> +<div class="breadcrumb">
> + <span ng-repeat="dir in path.split('/').slice(1)">
> + <a style="white-space: nowrap" href="#/agents/{{agent_id}}/browse?path={{
> encodeURIComponent(path.split('/').slice(0, $index + 1).join('/'))}}">
> + <span class="dir">/</span>
> {{dir}}
> </a>
> - </li>
> -</ol>
> + </span>
> +</div>
>
> <div class="alert alert-error hidden" id="alert">
> <button class="close" data-dismiss="alert">×</button>
> 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.
Posted by Tomasz Janiszewski <ja...@gmail.com>.
> On Maj 11, 2017, 8:35 rano, Tomasz Janiszewski wrote:
> > src/webui/master/static/browse.html
> > Lines 17-20 (original), 17-20 (patched)
> > <https://reviews.apache.org/r/58874/diff/3/?file=1710161#file1710161line17>
> >
> > How about usign `<span>` 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.
> > ```
> > <div class="breadcrunb">
> > <span ng-repeat="dir in path.split('/')">
> > <a href="#/agents/{{agent_id}}/browse?path={{
> > encodeURIComponent(path.split('/').slice(0, $index + 1).join('/'))
> > }}">
> > {{dir}}
> > </a>
> > <span>/</span>
> > </span>
> > </div>
> > ```
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 @@
</li>
</ol>
-<ol class="breadcrumb">
- <li ng-repeat="dir in path.split('/')">
- <a href="#/agents/{{agent_id}}/browse?path={{
+<div class="breadcrumb">
+ <span ng-repeat="dir in path.split('/').slice(1)">
+ <a style="white-space: nowrap" href="#/agents/{{agent_id}}/browse?path={{
encodeURIComponent(path.split('/').slice(0, $index + 1).join('/'))}}">
+ <span class="dir">/</span>
{{dir}}
</a>
- </li>
-</ol>
+ </span>
+</div>
<div class="alert alert-error hidden" id="alert">
<button class="close" data-dismiss="alert">×</button>
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.
Posted by Tomasz Janiszewski <ja...@gmail.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/58874/#comment247820>
How about usign `<span>` 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.
```
<div class="breadcrunb">
<span ng-repeat="dir in path.split('/')">
<a href="#/agents/{{agent_id}}/browse?path={{
encodeURIComponent(path.split('/').slice(0, $index + 1).join('/'))
}}">
{{dir}}
</a>
<span>/</span>
</span>
</div>
```
- 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.
Posted by haosdent huang <ha...@apache.org>.
-----------------------------------------------------------
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.
Posted by haosdent huang <ha...@apache.org>.
-----------------------------------------------------------
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.
Posted by haosdent huang <ha...@apache.org>.
-----------------------------------------------------------
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.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
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.
Posted by haosdent huang <ha...@apache.org>.
> On May 11, 2017, 12:38 a.m., Benjamin Mahler wrote:
> > src/webui/master/static/browse.html
> > Lines 17-20 (original), 17-20 (patched)
> > <https://reviews.apache.org/r/58874/diff/3/?file=1710161#file1710161line17>
> >
> > I suspect, if my understanding is correct, you could do the following format:
> >
> > ```
> > <li ng-repeat="dir in path.split('/')">
> > <a href="#/agents/{{agent_id}}/browse?path={{
> > encodeURIComponent(path.split('/').slice(0, $index + 1).join('/'))
> > }}">{{dir}}</a>
> > <span class="copy-placeholder">/</span>
> > </li>
> > ```
> >
> > Does that also insert a space?
Yes, this still insert a space
```
')">
<a
```
Both `\n` and ` ` would convert to space in html.
- 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.
Posted by haosdent huang <ha...@apache.org>.
> 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.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/58874/#comment247781>
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 `</a>` and `</li>` tags?
How about this?
```
<!-- We want to ensure that if the user highlights the path breadcrumb,
and copies it, they will receive a /path/without/spaces that they
can then paste into a terminal, or elsewhere. In order to do this,
we have to ensure there is no whitespace within the <a> tag contents.
Also, we have to inject a hidden '/' character because the slashes
in the breadcrumb are not copied.
TODO(haosdent): Figure out why the breadcrumb's '/' characters are
not being copied.
-->
```
Is this accurate?
src/webui/master/static/browse.html
Lines 17-20 (original), 17-20 (patched)
<https://reviews.apache.org/r/58874/#comment247782>
I suspect, if my understanding is correct, you could do the following format:
```
<li ng-repeat="dir in path.split('/')">
<a href="#/agents/{{agent_id}}/browse?path={{
encodeURIComponent(path.split('/').slice(0, $index + 1).join('/'))
}}">{{dir}}</a>
<span class="copy-placeholder">/</span>
</li>
```
Does that also insert a space?
src/webui/master/static/css/mesos.css
Lines 179-181 (patched)
<https://reviews.apache.org/r/58874/#comment247778>
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.
Posted by haosdent huang <ha...@apache.org>.
-----------------------------------------------------------
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.
Posted by haosdent huang <ha...@apache.org>.
-----------------------------------------------------------
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.
Posted by haosdent huang <ha...@apache.org>.
> On May 3, 2017, 7:45 p.m., Benjamin Mahler wrote:
> > src/webui/master/static/browse.html
> > Lines 15-16 (original), 15-16 (patched)
> > <https://reviews.apache.org/r/58874/diff/2/?file=1706419#file1706419line15>
> >
> > 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.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/58874/#comment246836>
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)
<https://reviews.apache.org/r/58874/#comment246837>
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.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
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.
Posted by haosdent huang <ha...@apache.org>.
-----------------------------------------------------------
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.
Posted by haosdent huang <ha...@apache.org>.
-----------------------------------------------------------
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.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
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
>
>
Re: Review Request 58874: Stripped spaces between directory elements
in WebUI.
Posted by Tomasz Janiszewski <ja...@gmail.com>.
> On Maj 1, 2017, 8:14 po południu, Tomasz Janiszewski wrote:
> > src/webui/master/static/browse.html
> > Line 17 (original), 17 (patched)
> > <https://reviews.apache.org/r/58874/diff/1/?file=1703836#file1703836line17>
> >
> > Could you keep original formatting?
>
> haosdent huang wrote:
> Change to
>
> ```
> <li ng-repeat="dir in path.split('/')"><a
> href="#/agents/{{agent_id}}/browse?path={{
> encodeURIComponent(path.split('/').slice(0, $index + 1).join('/'))
> }}">{{dir}}</a><span class="copy-placeholder">/</span></li>
> ```
>
> 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)
> > <https://reviews.apache.org/r/58874/diff/1/?file=1703837#file1703837line180>
> >
> > 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.
Posted by haosdent huang <ha...@apache.org>.
> On May 1, 2017, 8:14 p.m., Tomasz Janiszewski wrote:
> > src/webui/master/static/browse.html
> > Line 17 (original), 17 (patched)
> > <https://reviews.apache.org/r/58874/diff/1/?file=1703836#file1703836line17>
> >
> > Could you keep original formatting?
Change to
```
<li ng-repeat="dir in path.split('/')"><a
href="#/agents/{{agent_id}}/browse?path={{
encodeURIComponent(path.split('/').slice(0, $index + 1).join('/'))
}}">{{dir}}</a><span class="copy-placeholder">/</span></li>
```
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)
> > <https://reviews.apache.org/r/58874/diff/1/?file=1703837#file1703837line180>
> >
> > 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.
Posted by Tomasz Janiszewski <ja...@gmail.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/58874/#comment246488>
Could you keep original formatting?
src/webui/master/static/css/mesos.css
Lines 180 (patched)
<https://reviews.apache.org/r/58874/#comment246487>
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
>
>