Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2884563002:
Fix compact infobar height to match the height of Contextual Search. (Closed)
DescriptionFix compact infobar height to match the height of Contextual Search.
BUG=721943
TBR=dfalcantara@chromium.org
Review-Url: https://codereview.chromium.org/2884563002
Cr-Commit-Position: refs/heads/master@{#473915}
Committed: https://chromium.googlesource.com/chromium/src/+/508b1baa57fe9c5f0751a873a3e2e9ac87cf1ffa
Patch Set 1 #
Total comments: 1
Patch Set 2 : 56dp #Messages
Total messages: 17 (7 generated)
Description was changed from ========== Fix compact infobar height to match the height of Contextual Search. BUG=721943 ========== to ========== Fix compact infobar height to match the height of Contextual Search. BUG=721943 TBR=dfalcantara@chromium.org ==========
googleo@chromium.org changed reviewers: + mdjones@chromium.org
googleo@chromium.org changed reviewers: + twellington@chromium.org - mdjones@chromium.org
small change for the compact infobar height as request from PM
https://codereview.chromium.org/2884563002/diff/1/chrome/android/java/res/val... File chrome/android/java/res/values/dimens.xml (right): https://codereview.chromium.org/2884563002/diff/1/chrome/android/java/res/val... chrome/android/java/res/values/dimens.xml:101: <dimen name="infobar_compact_size">50dp</dimen> Contextual search maches the toolbar height; in non-Chrome Home it is 56dp (@dimen/overlay_panel_bar_height). Should this be 56dp too?
+Yana On Fri, 19 May 2017 at 9:43 am, <twellington@chromium.org> wrote: > > > https://codereview.chromium.org/2884563002/diff/1/chrome/android/java/res/val... > File chrome/android/java/res/values/dimens.xml (right): > > > https://codereview.chromium.org/2884563002/diff/1/chrome/android/java/res/val... > chrome/android/java/res/values/dimens.xml:101: <dimen > name="infobar_compact_size">50dp</dimen> > Contextual search maches the toolbar height; in non-Chrome Home it is > 56dp (@dimen/overlay_panel_bar_height). Should this be 56dp too? > > https://codereview.chromium.org/2884563002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
+ Bruno to confirm - this is with regard to changing height of infobar to match contextual search. I thought it was 50? Also, is it different in the Chrome home vs non-Chrome Home? If so we should probably go with Chrome Home since that's shipping in 60 too right? On Thu, May 18, 2017 at 4:50 PM, Leo Zhang <googleo@chromium.org> wrote: > +Yana > > On Fri, 19 May 2017 at 9:43 am, <twellington@chromium.org> wrote: > >> >> https://codereview.chromium.org/2884563002/diff/1/chrome/and >> roid/java/res/values/dimens.xml >> File chrome/android/java/res/values/dimens.xml (right): >> >> https://codereview.chromium.org/2884563002/diff/1/chrome/and >> roid/java/res/values/dimens.xml#newcode101 >> chrome/android/java/res/values/dimens.xml:101: <dimen >> name="infobar_compact_size">50dp</dimen> >> Contextual search maches the toolbar height; in non-Chrome Home it is >> 56dp (@dimen/overlay_panel_bar_height). Should this be 56dp too? >> >> https://codereview.chromium.org/2884563002/ >> > -- Yana Yushkina Google, Inc. -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
The toolbar, and therefore Contextual Search bar, in Chrome Home and non-Chrome Home are both 56dp. The toolbar in Chrome Home used to be bigger, but we recently (a few weeks ago) shrunk it. Regards, Theresa On Thu, May 18, 2017 at 5:32 PM, Yana Yushkina <yyushkina@google.com> wrote: > + Bruno to confirm - this is with regard to changing height of infobar to > match contextual search. I thought it was 50? Also, is it different in the > Chrome home vs non-Chrome Home? If so we should probably go with Chrome > Home since that's shipping in 60 too right? > > On Thu, May 18, 2017 at 4:50 PM, Leo Zhang <googleo@chromium.org> wrote: > >> +Yana >> >> On Fri, 19 May 2017 at 9:43 am, <twellington@chromium.org> wrote: >> >>> >>> https://codereview.chromium.org/2884563002/diff/1/chrome/and >>> roid/java/res/values/dimens.xml >>> File chrome/android/java/res/values/dimens.xml (right): >>> >>> https://codereview.chromium.org/2884563002/diff/1/chrome/and >>> roid/java/res/values/dimens.xml#newcode101 >>> chrome/android/java/res/values/dimens.xml:101: <dimen >>> name="infobar_compact_size">50dp</dimen> >>> Contextual search maches the toolbar height; in non-Chrome Home it is >>> 56dp (@dimen/overlay_panel_bar_height). Should this be 56dp too? >>> >>> https://codereview.chromium.org/2884563002/ >>> >> > > > -- > Yana Yushkina > Google, Inc. > > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
56dp it is, then! On Fri, May 19, 2017 at 7:57 AM Theresa Wellington <twellington@google.com> wrote: > The toolbar, and therefore Contextual Search bar, in Chrome Home and > non-Chrome Home are both 56dp. The toolbar in Chrome Home used to be > bigger, but we recently (a few weeks ago) shrunk it. > > Regards, > Theresa > > On Thu, May 18, 2017 at 5:32 PM, Yana Yushkina <yyushkina@google.com> > wrote: > >> + Bruno to confirm - this is with regard to changing height of infobar to >> match contextual search. I thought it was 50? Also, is it different in the >> Chrome home vs non-Chrome Home? If so we should probably go with Chrome >> Home since that's shipping in 60 too right? >> >> On Thu, May 18, 2017 at 4:50 PM, Leo Zhang <googleo@chromium.org> wrote: >> >>> +Yana >>> >>> On Fri, 19 May 2017 at 9:43 am, <twellington@chromium.org> wrote: >>> >>>> >>>> >>>> https://codereview.chromium.org/2884563002/diff/1/chrome/android/java/res/val... >>>> File chrome/android/java/res/values/dimens.xml (right): >>>> >>>> >>>> https://codereview.chromium.org/2884563002/diff/1/chrome/android/java/res/val... >>>> chrome/android/java/res/values/dimens.xml:101: <dimen >>>> name="infobar_compact_size">50dp</dimen> >>>> Contextual search maches the toolbar height; in non-Chrome Home it is >>>> 56dp (@dimen/overlay_panel_bar_height). Should this be 56dp too? >>>> >>>> https://codereview.chromium.org/2884563002/ >>>> >>> >> >> >> -- >> Yana Yushkina >> Google, Inc. >> >> > -- - B -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Changed size to 56dp, thanks for the review. PTAL
The CQ bit was checked by twellington@chromium.org
lgtm
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": 20001, "attempt_start_ts": 1495551263649330,
"parent_rev": "1bfba30b154f51cf18da3c2131764f7ef1e77aad", "commit_rev":
"95e999b6e5051c50a74120de85ec0054da1d132a"}
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1495551263649330,
"parent_rev": "f29aa27f64bf040b7e7fa7b4d894cb1a1ff31de6", "commit_rev":
"508b1baa57fe9c5f0751a873a3e2e9ac87cf1ffa"}
Message was sent while issue was closed.
Description was changed from ========== Fix compact infobar height to match the height of Contextual Search. BUG=721943 TBR=dfalcantara@chromium.org ========== to ========== Fix compact infobar height to match the height of Contextual Search. BUG=721943 TBR=dfalcantara@chromium.org Review-Url: https://codereview.chromium.org/2884563002 Cr-Commit-Position: refs/heads/master@{#473915} Committed: https://chromium.googlesource.com/chromium/src/+/508b1baa57fe9c5f0751a873a3e2... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/508b1baa57fe9c5f0751a873a3e2... |
