History: Use images instead of unicode characters.

History: Use images instead of unicode characters.

Unicode characters are font dependent so they may look differently on some
machines. This patch uses images for the arrows in grouping by domain and the
range next and previous buttons.

This patch also sets the domain name for grouped domains to a link so that users can tab to it and and pressing enter will expand.

Uses 2 images that will be uploaded in a different CL.

BUG=170690
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=183795

[Sergiu, 2013-02-07 18:22:42]

Take a look :)

[Patrick Dubroy, 2013-02-07 18:49:24]

https://codereview.chromium.org/12218058/diff/1/chrome/browser/resources/hist...
File chrome/browser/resources/history/history.css (right):

https://codereview.chromium.org/12218058/diff/1/chrome/browser/resources/hist...
chrome/browser/resources/history/history.css:239: #range-previous::before {
This should probably also be done with a background image. See how
chrome://settings puts the arrow on <select> items, for example.

https://codereview.chromium.org/12218058/diff/1/chrome/browser/resources/hist...
File chrome/browser/resources/history/history.js (right):

https://codereview.chromium.org/12218058/diff/1/chrome/browser/resources/hist...
chrome/browser/resources/history/history.js:868: var siteArrowLink =
siteArrow.appendChild(
I think the arrow should be a background image of siteDomainWrapper.

[Sergiu, 2013-02-07 19:34:23]

New better version,

+jhawkins@, especially for the minor added strings.

[James Hawkins, 2013-02-07 19:37:29]

https://codereview.chromium.org/12218058/diff/7001/chrome/browser/resources/h...
File chrome/browser/resources/history/history.css (right):

https://codereview.chromium.org/12218058/diff/7001/chrome/browser/resources/h...
chrome/browser/resources/history/history.css:241: margin-top: -1px;
Why the need for -1, 1?

https://codereview.chromium.org/12218058/diff/7001/chrome/browser/resources/h...
chrome/browser/resources/history/history.css:277: .site-domain a {
It's not being decorated like an anchor; is it going to act like an anchor?

https://codereview.chromium.org/12218058/diff/7001/chrome/browser/resources/h...
File chrome/browser/resources/history/history.html (right):

https://codereview.chromium.org/12218058/diff/7001/chrome/browser/resources/h...
chrome/browser/resources/history/history.html:53: <button id="range-previous"
disabled>
Is it styled differently when disabled?

https://codereview.chromium.org/12218058/diff/7001/chrome/browser/ui/webui/hi...
File chrome/browser/ui/webui/history_ui.cc (right):

https://codereview.chromium.org/12218058/diff/7001/chrome/browser/ui/webui/hi...
chrome/browser/ui/webui/history_ui.cc:146:
source->AddLocalizedString("rangenext", IDS_HISTORY_RANGE_NEXT);
rangeNext, rangePrevious

[Patrick Dubroy, 2013-02-07 19:38:43]

https://codereview.chromium.org/12218058/diff/7001/chrome/browser/resources/h...
File chrome/browser/resources/history/history.css (right):

https://codereview.chromium.org/12218058/diff/7001/chrome/browser/resources/h...
chrome/browser/resources/history/history.css:277: .site-domain a {
On 2013/02/07 19:37:29, James Hawkins wrote:
> It's not being decorated like an anchor; is it going to act like an anchor?

I think it should look and behave like a regular link, but clicking on it should
expand the items below it.

[James Hawkins, 2013-02-07 19:42:54]

https://codereview.chromium.org/12218058/diff/7001/chrome/browser/resources/h...
File chrome/browser/resources/history/history.css (right):

https://codereview.chromium.org/12218058/diff/7001/chrome/browser/resources/h...
chrome/browser/resources/history/history.css:277: .site-domain a {
On 2013/02/07 19:38:43, dubroy wrote:
> On 2013/02/07 19:37:29, James Hawkins wrote:
> > It's not being decorated like an anchor; is it going to act like an anchor?
> 
> I think it should look and behave like a regular link, but clicking on it
should
> expand the items below it.

Ah, so it's a link-button.  We have that in
chrome/browser/resources/widgets.css.

[Sergiu, 2013-02-08 10:01:17]

https://codereview.chromium.org/12218058/diff/7001/chrome/browser/resources/h...
File chrome/browser/resources/history/history.css (right):

https://codereview.chromium.org/12218058/diff/7001/chrome/browser/resources/h...
chrome/browser/resources/history/history.css:241: margin-top: -1px;
On 2013/02/07 19:37:29, James Hawkins wrote:
> Why the need for -1, 1?

Initially I thought it wasn't aligned properly but it was, removed them.

https://codereview.chromium.org/12218058/diff/7001/chrome/browser/resources/h...
chrome/browser/resources/history/history.css:277: .site-domain a {
On 2013/02/07 19:42:54, James Hawkins wrote:
> On 2013/02/07 19:38:43, dubroy wrote:
> > On 2013/02/07 19:37:29, James Hawkins wrote:
> > > It's not being decorated like an anchor; is it going to act like an
anchor?
> > 
> > I think it should look and behave like a regular link, but clicking on it
> should
> > expand the items below it.
> 
> Ah, so it's a link-button.  We have that in
> chrome/browser/resources/widgets.css.

I've set them to that, although I hope it's not too confusing for the users
(which could expect that clicking on the grouped "www.google.com" link takes
them to google.com while it just expands that group).

https://codereview.chromium.org/12218058/diff/7001/chrome/browser/resources/h...
File chrome/browser/resources/history/history.html (right):

https://codereview.chromium.org/12218058/diff/7001/chrome/browser/resources/h...
chrome/browser/resources/history/history.html:53: <button id="range-previous"
disabled>
On 2013/02/07 19:37:29, James Hawkins wrote:
> Is it styled differently when disabled?

Yes, the background on the button is slightly more washed out when it is
disabled.

https://codereview.chromium.org/12218058/diff/7001/chrome/browser/ui/webui/hi...
File chrome/browser/ui/webui/history_ui.cc (right):

https://codereview.chromium.org/12218058/diff/7001/chrome/browser/ui/webui/hi...
chrome/browser/ui/webui/history_ui.cc:146:
source->AddLocalizedString("rangenext", IDS_HISTORY_RANGE_NEXT);
On 2013/02/07 19:37:29, James Hawkins wrote:
> rangeNext, rangePrevious

I'll refactor all of them in a CL soon, keeping them consistent for now.

[James Hawkins, 2013-02-08 17:19:25]

https://codereview.chromium.org/12218058/diff/5002/chrome/browser/resources/h...
File chrome/browser/resources/history/history.js (right):

https://codereview.chromium.org/12218058/diff/5002/chrome/browser/resources/h...
chrome/browser/resources/history/history.js:871: createElementWithClassName('a',
'link-button'));
The thing about link-buttons is that you should not use anchors as the element
(you should use buttons).

[Patrick Dubroy, 2013-02-08 18:42:26]

https://codereview.chromium.org/12218058/diff/5002/chrome/browser/resources/h...
File chrome/browser/resources/history/history.css (right):

https://codereview.chromium.org/12218058/diff/5002/chrome/browser/resources/h...
chrome/browser/resources/history/history.css:273: color: black;
Shouldn't this be rgb(48, 57, 66), like the other links on the history page?

https://codereview.chromium.org/12218058/diff/5002/chrome/browser/resources/h...
chrome/browser/resources/history/history.css:274: text-decoration: none;
I think it should be decorated like a regular link.

[Sergiu, 2013-02-11 19:19:34]

Done, hope this is it. BTW there was a minor rebase error in the .js file but
it's corrected in the latest patch set.

https://codereview.chromium.org/12218058/diff/5002/chrome/browser/resources/h...
File chrome/browser/resources/history/history.css (right):

https://codereview.chromium.org/12218058/diff/5002/chrome/browser/resources/h...
chrome/browser/resources/history/history.css:273: color: black;
On 2013/02/08 18:42:26, dubroy wrote:
> Shouldn't this be rgb(48, 57, 66), like the other links on the history page?

Done.

https://codereview.chromium.org/12218058/diff/5002/chrome/browser/resources/h...
chrome/browser/resources/history/history.css:274: text-decoration: none;
On 2013/02/08 18:42:26, dubroy wrote:
> I think it should be decorated like a regular link.

This should have been removed when using link-button, did that now.

https://codereview.chromium.org/12218058/diff/5002/chrome/browser/resources/h...
File chrome/browser/resources/history/history.js (right):

https://codereview.chromium.org/12218058/diff/5002/chrome/browser/resources/h...
chrome/browser/resources/history/history.js:871: createElementWithClassName('a',
'link-button'));
On 2013/02/08 17:19:25, James Hawkins wrote:
> The thing about link-buttons is that you should not use anchors as the element
> (you should use buttons).

Didn't know that but I do know :). Replaced it with button tag.

[Sergiu, 2013-02-15 15:02:00]

> Didn't know that but I do know :). Replaced it with button tag.
s/know/now/

Put in the new images from Patrick and set the proper opacity on them.

[Patrick Dubroy, 2013-02-18 10:19:36]

https://codereview.chromium.org/12218058/diff/14001/chrome/browser/resources/...
File chrome/browser/resources/history/history.css (right):

https://codereview.chromium.org/12218058/diff/14001/chrome/browser/resources/...
chrome/browser/resources/history/history.css:274: -webkit-transform:
rotate(180deg);
BTW you can also do scaleX(-1) to mirror it. That might be better since it
avoids problems with the centering of the image.

https://codereview.chromium.org/12218058/diff/14001/chrome/browser/resources/...
chrome/browser/resources/history/history.css:314: color: #8F8F8F;
This should be specified in RGB.

https://codereview.chromium.org/12218058/diff/14001/chrome/browser/resources/...
File chrome/browser/resources/history/history.html (right):

https://codereview.chromium.org/12218058/diff/14001/chrome/browser/resources/...
chrome/browser/resources/history/history.html:54: <img
src="../disclosure_triangle_small.png" i18n-values="alt:rangeprevious">
Line is more than 80 chars.

[Sergiu, 2013-02-19 08:58:25]

https://codereview.chromium.org/12218058/diff/14001/chrome/browser/resources/...
File chrome/browser/resources/history/history.css (right):

https://codereview.chromium.org/12218058/diff/14001/chrome/browser/resources/...
chrome/browser/resources/history/history.css:274: -webkit-transform:
rotate(180deg);
On 2013/02/18 10:19:36, dubroy wrote:
> BTW you can also do scaleX(-1) to mirror it. That might be better since it
> avoids problems with the centering of the image.

Done.

https://codereview.chromium.org/12218058/diff/14001/chrome/browser/resources/...
chrome/browser/resources/history/history.css:314: color: #8F8F8F;
On 2013/02/18 10:19:36, dubroy wrote:
> This should be specified in RGB.

Done.

https://codereview.chromium.org/12218058/diff/14001/chrome/browser/resources/...
File chrome/browser/resources/history/history.html (right):

https://codereview.chromium.org/12218058/diff/14001/chrome/browser/resources/...
chrome/browser/resources/history/history.html:54: <img
src="../disclosure_triangle_small.png" i18n-values="alt:rangeprevious">
On 2013/02/18 10:19:36, dubroy wrote:
> Line is more than 80 chars.

Done.

[Patrick Dubroy, 2013-02-19 09:20:18]

lgtm

[Sergiu, 2013-02-19 12:34:12]

James, please take a look as well, thanks!

[James Hawkins, 2013-02-19 20:45:31]

lgtm

[commit-bot: I haz the power, 2013-02-21 10:41:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergiu@chromium.org/12218058/33001

[commit-bot: I haz the power, 2013-02-21 12:20:37]

Retried try job too often on linux_rel for step(s) browser_testsnet_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-02-21 12:25:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergiu@chromium.org/12218058/33001

[commit-bot: I haz the power, 2013-02-21 13:19:46]

Retried try job too often on linux_rel for step(s) browser_testsnet_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-02-21 13:26:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergiu@chromium.org/12218058/33001

[commit-bot: I haz the power, 2013-02-21 13:29:03]

Change committed as 183795