You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ross Allen <re...@gmail.com> on 2013/07/05 22:58:08 UTC
Review Request 12283: Replace "mesosDate" with "isoDate"
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12283/
-----------------------------------------------------------
Review request for mesos.
Repository: mesos
Description
-------
Dates now use HTML "title" attribute on <abbr> elements to let the browser do the hover work. Bootstrap already styles <abbr> with a dashed underline.
This removes the blue color from dates since they are not navigation.
Diffs
-----
src/webui/master/static/app.js 404516f8c760cdff564a9b1e8d2e0d74407daf67
src/webui/master/static/framework.html c3827e886351308dadd61fbe6a7570ad5d3d5110
src/webui/master/static/frameworks.html acb0eaa3ca8e893c92f4cfe887760d8a6f3ad182
src/webui/master/static/home.html 0074f318e87070fd2f5004f23a4d80d04a7047d1
src/webui/master/static/slave.html 9a52f907271e362d3dac127fc74e8c4a8581e43d
src/webui/master/static/slaves.html 508748f16c87a1ad2f3f9f4af33767165b61052c
Diff: https://reviews.apache.org/r/12283/diff/
Testing
-------
Thanks,
Ross Allen
Re: Review Request 12283: Replace "mesosDate" with "isoDate"
Posted by Ross Allen <re...@gmail.com>.
> On July 8, 2013, 4:09 p.m., Benjamin Hindman wrote:
> > src/webui/master/static/home.html, line 24
> > <https://reviews.apache.org/r/12283/diff/1/?file=318380#file318380line24>
> >
> > Move </abbr> to next line to be consistent with others?
Putting the closing tag on the same line is to prevent trailing spaces from being underlined.
In this example, the whitespace in the <abbr> is collapsed because there is no more content inside the parent. "USA" is underlined:
<div>
<abbr>
USA
</abbr>
</div>
In this example, which matches the commented case, the browser interprets the whitespace inside the <abbr> tag as meaningful. Since <abbr> has an underline in Bootstrap, "USA " gets underlined with a trailing space:
<div>
<abbr>
USA
</abbr>
is in the Northern hemisphere.
</div>
- Ross
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12283/#review22823
-----------------------------------------------------------
On July 5, 2013, 10:55 p.m., Ross Allen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12283/
> -----------------------------------------------------------
>
> (Updated July 5, 2013, 10:55 p.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Dates now use HTML "title" attribute on <abbr> elements to let the browser do the hover work. Bootstrap already styles <abbr> with a dashed underline.
>
> This removes the blue color from dates since they are not navigation.
>
>
> Diffs
> -----
>
> src/webui/master/static/app.js 404516f8c760cdff564a9b1e8d2e0d74407daf67
> src/webui/master/static/framework.html c3827e886351308dadd61fbe6a7570ad5d3d5110
> src/webui/master/static/frameworks.html acb0eaa3ca8e893c92f4cfe887760d8a6f3ad182
> src/webui/master/static/home.html 0074f318e87070fd2f5004f23a4d80d04a7047d1
> src/webui/master/static/slave.html 9a52f907271e362d3dac127fc74e8c4a8581e43d
> src/webui/master/static/slaves.html 508748f16c87a1ad2f3f9f4af33767165b61052c
>
> Diff: https://reviews.apache.org/r/12283/diff/
>
>
> Testing
> -------
>
> Viewed all pages with "isoDate" uses to confirm rendering.
>
> Hovered over dates in on each page to confirm "title" attribute showed correct time.
>
>
> Thanks,
>
> Ross Allen
>
>
Re: Review Request 12283: Replace "mesosDate" with "isoDate"
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On July 8, 2013, 4:09 p.m., Benjamin Hindman wrote:
> > src/webui/master/static/home.html, line 24
> > <https://reviews.apache.org/r/12283/diff/1/?file=318380#file318380line24>
> >
> > Move </abbr> to next line to be consistent with others?
>
> Ross Allen wrote:
> Putting the closing tag on the same line is to prevent trailing spaces from being underlined.
>
> In this example, the whitespace in the <abbr> is collapsed because there is no more content inside the parent. "USA" is underlined:
>
> <div>
> <abbr>
> USA
> </abbr>
> </div>
>
> In this example, which matches the commented case, the browser interprets the whitespace inside the <abbr> tag as meaningful. Since <abbr> has an underline in Bootstrap, "USA " gets underlined with a trailing space:
>
> <div>
> <abbr>
> USA
> </abbr>
> is in the Northern hemisphere.
> </div>
Awesome, thanks for the explanation!
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12283/#review22823
-----------------------------------------------------------
On July 5, 2013, 10:55 p.m., Ross Allen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12283/
> -----------------------------------------------------------
>
> (Updated July 5, 2013, 10:55 p.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Dates now use HTML "title" attribute on <abbr> elements to let the browser do the hover work. Bootstrap already styles <abbr> with a dashed underline.
>
> This removes the blue color from dates since they are not navigation.
>
>
> Diffs
> -----
>
> src/webui/master/static/app.js 404516f8c760cdff564a9b1e8d2e0d74407daf67
> src/webui/master/static/framework.html c3827e886351308dadd61fbe6a7570ad5d3d5110
> src/webui/master/static/frameworks.html acb0eaa3ca8e893c92f4cfe887760d8a6f3ad182
> src/webui/master/static/home.html 0074f318e87070fd2f5004f23a4d80d04a7047d1
> src/webui/master/static/slave.html 9a52f907271e362d3dac127fc74e8c4a8581e43d
> src/webui/master/static/slaves.html 508748f16c87a1ad2f3f9f4af33767165b61052c
>
> Diff: https://reviews.apache.org/r/12283/diff/
>
>
> Testing
> -------
>
> Viewed all pages with "isoDate" uses to confirm rendering.
>
> Hovered over dates in on each page to confirm "title" attribute showed correct time.
>
>
> Thanks,
>
> Ross Allen
>
>
Re: Review Request 12283: Replace "mesosDate" with "isoDate"
Posted by Ross Allen <re...@gmail.com>.
> On July 8, 2013, 4:09 p.m., Benjamin Hindman wrote:
> > I like the simplification. A little context here: we originally used the popovers because we wanted to be able to copy the date for searching the logs. ;) At this point in time that hasn't really been necessary, so I'm fine with shipping this, but we might want to think about other ways to easily copy dates (or other cumbersome strings, like IDs) to make it easy to search in the logs.
> >
> > If others have found the popovers useful for log searching please speak up now!
>
> Ross Allen wrote:
> I figured copying/pasting was likely the reason behind the popovers and will look at a better solution because of that.
>
> Github prints only shortened SHAs for commits and adds a "Copy SHA" button wherever it shows shortened SHAs. I like that approach with the caveat that it requires Flash and therefore would not work on devices that don't support Flash.
>
> Thoughts?
Also, does that mean the "mesosDate" function printed dates the same way they appear in logs? Are dates in the American format in the logs? It would be great to keep them consistent.
- Ross
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12283/#review22823
-----------------------------------------------------------
On July 5, 2013, 10:55 p.m., Ross Allen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12283/
> -----------------------------------------------------------
>
> (Updated July 5, 2013, 10:55 p.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Dates now use HTML "title" attribute on <abbr> elements to let the browser do the hover work. Bootstrap already styles <abbr> with a dashed underline.
>
> This removes the blue color from dates since they are not navigation.
>
>
> Diffs
> -----
>
> src/webui/master/static/app.js 404516f8c760cdff564a9b1e8d2e0d74407daf67
> src/webui/master/static/framework.html c3827e886351308dadd61fbe6a7570ad5d3d5110
> src/webui/master/static/frameworks.html acb0eaa3ca8e893c92f4cfe887760d8a6f3ad182
> src/webui/master/static/home.html 0074f318e87070fd2f5004f23a4d80d04a7047d1
> src/webui/master/static/slave.html 9a52f907271e362d3dac127fc74e8c4a8581e43d
> src/webui/master/static/slaves.html 508748f16c87a1ad2f3f9f4af33767165b61052c
>
> Diff: https://reviews.apache.org/r/12283/diff/
>
>
> Testing
> -------
>
> Viewed all pages with "isoDate" uses to confirm rendering.
>
> Hovered over dates in on each page to confirm "title" attribute showed correct time.
>
>
> Thanks,
>
> Ross Allen
>
>
Re: Review Request 12283: Replace "mesosDate" with "isoDate"
Posted by Ross Allen <re...@gmail.com>.
> On July 8, 2013, 4:09 p.m., Benjamin Hindman wrote:
> > I like the simplification. A little context here: we originally used the popovers because we wanted to be able to copy the date for searching the logs. ;) At this point in time that hasn't really been necessary, so I'm fine with shipping this, but we might want to think about other ways to easily copy dates (or other cumbersome strings, like IDs) to make it easy to search in the logs.
> >
> > If others have found the popovers useful for log searching please speak up now!
I figured copying/pasting was likely the reason behind the popovers and will look at a better solution because of that.
Github prints only shortened SHAs for commits and adds a "Copy SHA" button wherever it shows shortened SHAs. I like that approach with the caveat that it requires Flash and therefore would not work on devices that don't support Flash.
Thoughts?
- Ross
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12283/#review22823
-----------------------------------------------------------
On July 5, 2013, 10:55 p.m., Ross Allen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12283/
> -----------------------------------------------------------
>
> (Updated July 5, 2013, 10:55 p.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Dates now use HTML "title" attribute on <abbr> elements to let the browser do the hover work. Bootstrap already styles <abbr> with a dashed underline.
>
> This removes the blue color from dates since they are not navigation.
>
>
> Diffs
> -----
>
> src/webui/master/static/app.js 404516f8c760cdff564a9b1e8d2e0d74407daf67
> src/webui/master/static/framework.html c3827e886351308dadd61fbe6a7570ad5d3d5110
> src/webui/master/static/frameworks.html acb0eaa3ca8e893c92f4cfe887760d8a6f3ad182
> src/webui/master/static/home.html 0074f318e87070fd2f5004f23a4d80d04a7047d1
> src/webui/master/static/slave.html 9a52f907271e362d3dac127fc74e8c4a8581e43d
> src/webui/master/static/slaves.html 508748f16c87a1ad2f3f9f4af33767165b61052c
>
> Diff: https://reviews.apache.org/r/12283/diff/
>
>
> Testing
> -------
>
> Viewed all pages with "isoDate" uses to confirm rendering.
>
> Hovered over dates in on each page to confirm "title" attribute showed correct time.
>
>
> Thanks,
>
> Ross Allen
>
>
Re: Review Request 12283: Replace "mesosDate" with "isoDate"
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On July 8, 2013, 4:09 p.m., Benjamin Hindman wrote:
> > I like the simplification. A little context here: we originally used the popovers because we wanted to be able to copy the date for searching the logs. ;) At this point in time that hasn't really been necessary, so I'm fine with shipping this, but we might want to think about other ways to easily copy dates (or other cumbersome strings, like IDs) to make it easy to search in the logs.
> >
> > If others have found the popovers useful for log searching please speak up now!
>
> Ross Allen wrote:
> I figured copying/pasting was likely the reason behind the popovers and will look at a better solution because of that.
>
> Github prints only shortened SHAs for commits and adds a "Copy SHA" button wherever it shows shortened SHAs. I like that approach with the caveat that it requires Flash and therefore would not work on devices that don't support Flash.
>
> Thoughts?
>
> Ross Allen wrote:
> Also, does that mean the "mesosDate" function printed dates the same way they appear in logs? Are dates in the American format in the logs? It would be great to keep them consistent.
I'm not too concerned with the flash requirement, especially if we can make it such that the only lost functionality on these devices is the copying (that is, while I could see someone quickly looking at the webui via a non-flash device, I don't see them copying the date and grepping logs on the same device!). We had talked about this before but nobody had sufficient experience to take it on.
I'm happy to see isoDate instead of mesosDate. The logs (from the glog library) don't actually include the year, just MMDD HH:MM:SS. We typically just copied the time (if anything), and manually typed out the month and day.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12283/#review22823
-----------------------------------------------------------
On July 5, 2013, 10:55 p.m., Ross Allen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12283/
> -----------------------------------------------------------
>
> (Updated July 5, 2013, 10:55 p.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Dates now use HTML "title" attribute on <abbr> elements to let the browser do the hover work. Bootstrap already styles <abbr> with a dashed underline.
>
> This removes the blue color from dates since they are not navigation.
>
>
> Diffs
> -----
>
> src/webui/master/static/app.js 404516f8c760cdff564a9b1e8d2e0d74407daf67
> src/webui/master/static/framework.html c3827e886351308dadd61fbe6a7570ad5d3d5110
> src/webui/master/static/frameworks.html acb0eaa3ca8e893c92f4cfe887760d8a6f3ad182
> src/webui/master/static/home.html 0074f318e87070fd2f5004f23a4d80d04a7047d1
> src/webui/master/static/slave.html 9a52f907271e362d3dac127fc74e8c4a8581e43d
> src/webui/master/static/slaves.html 508748f16c87a1ad2f3f9f4af33767165b61052c
>
> Diff: https://reviews.apache.org/r/12283/diff/
>
>
> Testing
> -------
>
> Viewed all pages with "isoDate" uses to confirm rendering.
>
> Hovered over dates in on each page to confirm "title" attribute showed correct time.
>
>
> Thanks,
>
> Ross Allen
>
>
Re: Review Request 12283: Replace "mesosDate" with "isoDate"
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12283/#review22823
-----------------------------------------------------------
Ship it!
I like the simplification. A little context here: we originally used the popovers because we wanted to be able to copy the date for searching the logs. ;) At this point in time that hasn't really been necessary, so I'm fine with shipping this, but we might want to think about other ways to easily copy dates (or other cumbersome strings, like IDs) to make it easy to search in the logs.
If others have found the popovers useful for log searching please speak up now!
src/webui/master/static/home.html
<https://reviews.apache.org/r/12283/#comment46536>
Move </abbr> to next line to be consistent with others?
src/webui/master/static/slave.html
<https://reviews.apache.org/r/12283/#comment46537>
Given that this was consistent with above which also had a 'by ...' suffix, perhaps an explanation about why having </abbr> not on a newline here but everywhere else? Just a style thing?
- Benjamin Hindman
On July 5, 2013, 10:55 p.m., Ross Allen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12283/
> -----------------------------------------------------------
>
> (Updated July 5, 2013, 10:55 p.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Dates now use HTML "title" attribute on <abbr> elements to let the browser do the hover work. Bootstrap already styles <abbr> with a dashed underline.
>
> This removes the blue color from dates since they are not navigation.
>
>
> Diffs
> -----
>
> src/webui/master/static/app.js 404516f8c760cdff564a9b1e8d2e0d74407daf67
> src/webui/master/static/framework.html c3827e886351308dadd61fbe6a7570ad5d3d5110
> src/webui/master/static/frameworks.html acb0eaa3ca8e893c92f4cfe887760d8a6f3ad182
> src/webui/master/static/home.html 0074f318e87070fd2f5004f23a4d80d04a7047d1
> src/webui/master/static/slave.html 9a52f907271e362d3dac127fc74e8c4a8581e43d
> src/webui/master/static/slaves.html 508748f16c87a1ad2f3f9f4af33767165b61052c
>
> Diff: https://reviews.apache.org/r/12283/diff/
>
>
> Testing
> -------
>
> Viewed all pages with "isoDate" uses to confirm rendering.
>
> Hovered over dates in on each page to confirm "title" attribute showed correct time.
>
>
> Thanks,
>
> Ross Allen
>
>
Re: Review Request 12283: Replace "mesosDate" with "isoDate"
Posted by Ross Allen <re...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12283/
-----------------------------------------------------------
(Updated July 5, 2013, 10:55 p.m.)
Review request for mesos.
Repository: mesos
Description
-------
Dates now use HTML "title" attribute on <abbr> elements to let the browser do the hover work. Bootstrap already styles <abbr> with a dashed underline.
This removes the blue color from dates since they are not navigation.
Diffs
-----
src/webui/master/static/app.js 404516f8c760cdff564a9b1e8d2e0d74407daf67
src/webui/master/static/framework.html c3827e886351308dadd61fbe6a7570ad5d3d5110
src/webui/master/static/frameworks.html acb0eaa3ca8e893c92f4cfe887760d8a6f3ad182
src/webui/master/static/home.html 0074f318e87070fd2f5004f23a4d80d04a7047d1
src/webui/master/static/slave.html 9a52f907271e362d3dac127fc74e8c4a8581e43d
src/webui/master/static/slaves.html 508748f16c87a1ad2f3f9f4af33767165b61052c
Diff: https://reviews.apache.org/r/12283/diff/
Testing (updated)
-------
Viewed all pages with "isoDate" uses to confirm rendering.
Hovered over dates in on each page to confirm "title" attribute showed correct time.
Thanks,
Ross Allen