Description was changed from ========== MD Settings: Fix button styles in several places. Some buttons ...
3 years, 10 months ago
(2017-02-01 00:44:28 UTC)
#1
Description was changed from
==========
MD Settings: Fix button styles in several places.
Some buttons were missing the "secondary-button"/"cancel-buton" class.
BUG=684152
==========
to
==========
MD Settings: Fix button styles in several places.
Some buttons were missing the "secondary-button"/"cancel-buton" class.
BUG=684152
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========
scottchen
Description was changed from ========== MD Settings: Fix button styles in several places. Some buttons ...
3 years, 10 months ago
(2017-02-01 00:45:04 UTC)
#2
Description was changed from
==========
MD Settings: Fix button styles in several places.
Some buttons were missing the "secondary-button"/"cancel-buton" class.
BUG=684152
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========
to
==========
MD Settings: Fix button styles in several places.
Some buttons were missing the "secondary-button"/"cancel-buton" class.
BUG=684152
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========
Description was changed from ========== MD Settings: Fix button styles in several places. Some buttons ...
3 years, 10 months ago
(2017-02-01 02:22:36 UTC)
#4
Description was changed from
==========
MD Settings: Fix button styles in several places.
Some buttons were missing the "secondary-button"/"cancel-buton" class.
BUG=684152
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========
to
==========
MD Settings: Fix styles in several places.
- Some buttons were missing the "secondary-button"/"cancel-buton" class.
- removing rule-lines
BUG=684152
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========
scottchen
Description was changed from ========== MD Settings: Fix styles in several places. - Some buttons ...
3 years, 10 months ago
(2017-02-01 02:22:58 UTC)
#5
Description was changed from
==========
MD Settings: Fix styles in several places.
- Some buttons were missing the "secondary-button"/"cancel-buton" class.
- removing rule-lines
BUG=684152
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========
to
==========
MD Settings: Fix styles in several places.
- Some buttons were missing the "secondary-button"/"cancel-button" class.
- removing rule-lines
BUG=684152
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========
scottchen
Description was changed from ========== MD Settings: Fix styles in several places. - Some buttons ...
3 years, 10 months ago
(2017-02-01 02:40:17 UTC)
#6
Description was changed from
==========
MD Settings: Fix styles in several places.
- Some buttons were missing the "secondary-button"/"cancel-button" class.
- removing rule-lines
BUG=684152
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========
to
==========
MD Settings: Fix styles in several places.
- Some buttons were missing the "secondary-button"/"cancel-button" class.
- removing rule-lines
- remove unnecessary learn-more links
BUG=684152
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========
scottchen
Description was changed from ========== MD Settings: Fix styles in several places. - Some buttons ...
3 years, 10 months ago
(2017-02-01 22:35:31 UTC)
#7
Description was changed from
==========
MD Settings: Fix styles in several places.
- Some buttons were missing the "secondary-button"/"cancel-button" class.
- removing rule-lines
- remove unnecessary learn-more links
BUG=684152
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========
to
==========
MD Settings: Fix styles in several places.
- Some buttons were missing the "secondary-button"/"cancel-button" class.
- removing rule-lines
- remove unnecessary controls
- changing text button to icon button
BUG=684152
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========
There's several misc. visual fixes in here, most of which are just adding class or ...
3 years, 10 months ago
(2017-02-01 22:37:09 UTC)
#9
There's several misc. visual fixes in here, most of which are just adding class
or editing css value.
The one that's more notable is the controlled-button change to support icon:
http://imgur.com/a/XVPDv
tommycli
https://codereview.chromium.org/2668163002/diff/70001/chrome/browser/resources/settings/controls/controlled_button.html File chrome/browser/resources/settings/controls/controlled_button.html (right): https://codereview.chromium.org/2668163002/diff/70001/chrome/browser/resources/settings/controls/controlled_button.html#newcode42 chrome/browser/resources/settings/controls/controlled_button.html:42: <template is="dom-if" if="[[iconClass]]"> The controlled-button changes seem odd. I ...
3 years, 10 months ago
(2017-02-01 22:54:49 UTC)
#10
Nice, thanks for working on this. https://codereview.chromium.org/2668163002/diff/70001/chrome/browser/resources/settings/downloads_page/downloads_page.html File chrome/browser/resources/settings/downloads_page/downloads_page.html (left): https://codereview.chromium.org/2668163002/diff/70001/chrome/browser/resources/settings/downloads_page/downloads_page.html#oldcode27 chrome/browser/resources/settings/downloads_page/downloads_page.html:27: $i18n{changeDownloadLocation} If this ...
3 years, 10 months ago
(2017-02-01 23:06:23 UTC)
#11
Description was changed from ========== MD Settings: Fix styles in several places. - Some buttons ...
3 years, 10 months ago
(2017-02-02 01:17:31 UTC)
#13
Description was changed from
==========
MD Settings: Fix styles in several places.
- Some buttons were missing the "secondary-button"/"cancel-button" class.
- removing rule-lines
- remove unnecessary controls
- changing text button to icon button
BUG=684152
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========
to
==========
MD Settings: Fix styles in several places.
- Some buttons were missing the "secondary-button"/"cancel-button" class.
- removing rule-lines
- remove unnecessary controls
BUG=684152
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========
3 years, 10 months ago
(2017-02-02 20:02:14 UTC)
#20
https://codereview.chromium.org/2668163002/diff/70001/chrome/browser/resource...
File
chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html
(right):
https://codereview.chromium.org/2668163002/diff/70001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html:130:
<span id="passwordExceptionsList" class="vertical-list">
On 2017/02/02 01:17:14, scottchen wrote:
> On 2017/02/01 23:06:23, dschuyler wrote:
> > May I see a screen shot or demo of what this is changing?
>
> http://imgur.com/a/OOaTg There used to not be borders in between the items
Thanks.
Please consider merging the attributes from the span on line 130 into the div on
line 129 (and removing the span). Then add
hidden$="[[!hasSome_(passwordExceptions)]]" in that div to be clear that and
line 146 form a mutual exclusion (aka if/else).
[The empty span would work in a similar way, but it's implicit (which is harder
to see/read)].
https://codereview.chromium.org/2668163002/diff/150001/chrome/browser/resourc...
File
chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html
(right):
https://codereview.chromium.org/2668163002/diff/150001/chrome/browser/resourc...
chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html:136:
class="selectable">[[item.exceptionUrl]]</a>
Sorry I didn't notice what this might imply before. Was this two-line to allow
long urls to wrap(?) -- maybe this should .text-elide (or verify that it still
wraps and looks good without the .two-line).
3 years, 10 months ago
(2017-02-02 20:31:41 UTC)
#21
https://codereview.chromium.org/2668163002/diff/70001/chrome/browser/resource...
File
chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html
(right):
https://codereview.chromium.org/2668163002/diff/70001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html:130:
<span id="passwordExceptionsList" class="vertical-list">
On 2017/02/02 20:02:14, dschuyler wrote:
> On 2017/02/02 01:17:14, scottchen wrote:
> > On 2017/02/01 23:06:23, dschuyler wrote:
> > > May I see a screen shot or demo of what this is changing?
> >
> > http://imgur.com/a/OOaTg There used to not be borders in between the items
>
> Thanks.
>
> Please consider merging the attributes from the span on line 130 into the div
on
> line 129 (and removing the span). Then add
> hidden$="[[!hasSome_(passwordExceptions)]]" in that div to be clear that and
> line 146 form a mutual exclusion (aka if/else).
>
> [The empty span would work in a similar way, but it's implicit (which is
harder
> to see/read)].
So I tried that actually, but then realized line 146 is actually inside the div
on line 129, since the "not found" text needs to be displayed as a "list-item"
inside the empty list container, and currently the unit test depends pretty
heavily on counting the number of nodes inside #passwordExceptionList..
I'll see what I can do to make this less nested.
https://codereview.chromium.org/2668163002/diff/150001/chrome/browser/resourc...
File
chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html
(right):
https://codereview.chromium.org/2668163002/diff/150001/chrome/browser/resourc...
chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html:136:
class="selectable">[[item.exceptionUrl]]</a>
On 2017/02/02 20:02:14, dschuyler wrote:
> Sorry I didn't notice what this might imply before. Was this two-line to allow
> long urls to wrap(?) -- maybe this should .text-elide (or verify that it still
> wraps and looks good without the .two-line).
It originally did not wrap either, so to be honest I don't think the .two-line
did anything. Will look into eliding.
scottchen
https://codereview.chromium.org/2668163002/diff/170001/chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html File chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html (right): https://codereview.chromium.org/2668163002/diff/170001/chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html#newcode144 chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html:144: <div id="noExceptionsLabel" class="list-item message-item" dschuyler@ I had to add ...
3 years, 10 months ago
(2017-02-02 22:30:20 UTC)
#22
CQ is committing da patch. Bot data: {"patchset_id": 190001, "attempt_start_ts": 1486086110955400, "parent_rev": "4116d5e5f6fd92ae9f75cc2bfe158068232c6dbe", "commit_rev": "d8126246d566c209b04c3beb9a88b9258d37461c"}
3 years, 10 months ago
(2017-02-03 04:08:41 UTC)
#30
CQ is committing da patch.
Bot data: {"patchset_id": 190001, "attempt_start_ts": 1486086110955400,
"parent_rev": "4116d5e5f6fd92ae9f75cc2bfe158068232c6dbe", "commit_rev":
"d8126246d566c209b04c3beb9a88b9258d37461c"}
commit-bot: I haz the power
Description was changed from ========== MD Settings: Fix styles in several places. - Some buttons ...
3 years, 10 months ago
(2017-02-03 04:09:46 UTC)
#31
Message was sent while issue was closed.
Description was changed from
==========
MD Settings: Fix styles in several places.
- Some buttons were missing the "secondary-button"/"cancel-button" class.
- removing rule-lines
- remove unnecessary controls
BUG=684152
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========
to
==========
MD Settings: Fix styles in several places.
- Some buttons were missing the "secondary-button"/"cancel-button" class.
- removing rule-lines
- remove unnecessary controls
BUG=684152
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2668163002
Cr-Commit-Position: refs/heads/master@{#447923}
Committed:
https://chromium.googlesource.com/chromium/src/+/d8126246d566c209b04c3beb9a88...
==========
commit-bot: I haz the power
Committed patchset #11 (id:190001) as https://chromium.googlesource.com/chromium/src/+/d8126246d566c209b04c3beb9a88b9258d37461c
3 years, 10 months ago
(2017-02-03 04:09:47 UTC)
#32
Issue 2668163002: MD Settings: Fix styles in several places.
(Closed)
Created 3 years, 10 months ago by scottchen
Modified 3 years, 10 months ago
Reviewers: dschuyler, tommycli
Base URL:
Comments: 17