Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2974583002:
MD Settings: Tweak search bubble opacity and size. (Closed)
|
Created:
3 years, 5 months ago by dpapad Modified:
3 years, 5 months ago Reviewers:
dschuyler CC:
arv+watch_chromium.org, chromium-reviews, michaelpg+watch-md-settings_chromium.org, michaelpg+watch-md-ui_chromium.org, stevenjb+watch-md-settings_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionMD Settings: Tweak search bubble opacity and size.
BUG=732795
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2974583002
Cr-Commit-Position: refs/heads/master@{#485034}
Committed: https://chromium.googlesource.com/chromium/src/+/b36f4aaf404427cef20af39e7b657f9afe942ae0
Patch Set 1 #
Total comments: 1
Messages
Total messages: 16 (9 generated)
Description was changed from ========== MD Settings: Tweak search bubble opacity and size. BUG=732795 ========== to ========== MD Settings: Tweak search bubble opacity and size. BUG=732795 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by dpapad@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
dpapad@chromium.org changed reviewers: + dschuyler@chromium.org
Before/after screenshots at http://imgur.com/a/Gq8IH.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
LGTM because I think the overdraw (over-sized) diamond is unlikely to be any issue. https://codereview.chromium.org/2974583002/diff/1/chrome/browser/resources/se... File chrome/browser/resources/settings/settings_shared_css.html (right): https://codereview.chromium.org/2974583002/diff/1/chrome/browser/resources/se... chrome/browser/resources/settings/settings_shared_css.html:400: left: calc(50% - 5px); Maybe: The width of a diamond with 10px sides is about 7.0711px or about a shift of (50% - 3.5355px). Another option is to reduce the side length down from 10px. Though not a big deal if showing part of the diamond looks good as-is. It's just that more than half the diamond is tucked under.
On 2017/07/07 19:26:08, dschuyler wrote: > LGTM because I think the overdraw (over-sized) diamond is unlikely to be any > issue. > > https://codereview.chromium.org/2974583002/diff/1/chrome/browser/resources/se... > File chrome/browser/resources/settings/settings_shared_css.html (right): > > https://codereview.chromium.org/2974583002/diff/1/chrome/browser/resources/se... > chrome/browser/resources/settings/settings_shared_css.html:400: left: calc(50% - > 5px); > Maybe: The width of a diamond with 10px sides is about 7.0711px or about a shift > of (50% - 3.5355px). Another option is to reduce the side length down from 10px. > Though not a big deal if showing part of the diamond looks good as-is. It's just > that more than half the diamond is tucked under. :( and I entered the wrong numbers. Silly of me. The width is larger not smaller.
On 2017/07/07 19:27:23, dschuyler wrote: > On 2017/07/07 19:26:08, dschuyler wrote: > > LGTM because I think the overdraw (over-sized) diamond is unlikely to be any > > issue. > > > > > https://codereview.chromium.org/2974583002/diff/1/chrome/browser/resources/se... > > File chrome/browser/resources/settings/settings_shared_css.html (right): > > > > > https://codereview.chromium.org/2974583002/diff/1/chrome/browser/resources/se... > > chrome/browser/resources/settings/settings_shared_css.html:400: left: calc(50% > - > > 5px); > > Maybe: The width of a diamond with 10px sides is about 7.0711px or about a > shift > > of (50% - 3.5355px). Another option is to reduce the side length down from > 10px. > > Though not a big deal if showing part of the diamond looks good as-is. It's > just > > that more than half the diamond is tucked under. > > :( and I entered the wrong numbers. Silly of me. The width is larger not > smaller. The width of diamond with 10px sides is 14.142135px. And a shift of half that would center it. Sorry for the bogus numbers above.
On 2017/07/07 at 19:29:44, dschuyler wrote: > On 2017/07/07 19:27:23, dschuyler wrote: > > On 2017/07/07 19:26:08, dschuyler wrote: > > > LGTM because I think the overdraw (over-sized) diamond is unlikely to be any > > > issue. > > > > > > > > https://codereview.chromium.org/2974583002/diff/1/chrome/browser/resources/se... > > > File chrome/browser/resources/settings/settings_shared_css.html (right): > > > > > > > > https://codereview.chromium.org/2974583002/diff/1/chrome/browser/resources/se... > > > chrome/browser/resources/settings/settings_shared_css.html:400: left: calc(50% > > - > > > 5px); > > > Maybe: The width of a diamond with 10px sides is about 7.0711px or about a > > shift > > > of (50% - 3.5355px). Another option is to reduce the side length down from > > 10px. > > > Though not a big deal if showing part of the diamond looks good as-is. It's > > just > > > that more than half the diamond is tucked under. > > > > :( and I entered the wrong numbers. Silly of me. The width is larger not > > smaller. > > The width of diamond with 10px sides is 14.142135px. And a shift of half that would center it. > Sorry for the bogus numbers above. Summarizing offline discussion, the square seems to be first positioned, then rotated from its center point, which makes 50% - 5px position it exactly in the middle of the top/bottom edge of the search bubble.
The CQ bit was checked by dpapad@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1499459825129670, "parent_rev":
"2fe03e902371cafe3421893bf64f0f12f97136e0", "commit_rev":
"b36f4aaf404427cef20af39e7b657f9afe942ae0"}
Message was sent while issue was closed.
Description was changed from ========== MD Settings: Tweak search bubble opacity and size. BUG=732795 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== MD Settings: Tweak search bubble opacity and size. BUG=732795 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2974583002 Cr-Commit-Position: refs/heads/master@{#485034} Committed: https://chromium.googlesource.com/chromium/src/+/b36f4aaf404427cef20af39e7b65... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/b36f4aaf404427cef20af39e7b65... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
