New issue
Advanced search Search tips

Issue 794196 link

Starred by 2 users
Status: Fixed
Owner:
bsep@chromium.org
Closed: Jan 2018
Cc:
tapted@chromium.org
bettes@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

[MacViews] The text at the bottom part of the Bookmarks Star Bubble could be vertically centered

Project Member Reported by meh...@chromium.org, Dec 12 2017 
Chrome Version: 65.0.3292.0 canary (64-Bit)
OS: macOS 10.12.6

What steps will reproduce the problem?
(1) Visit a page
(2) Click on the Bookmarks Star icon in the Omnibox, so that the Bubble appears
(3) Take a look at the Bottom part of the Bubble

What is the expected result?
The text could be vertically centered in the box.

What happens instead?
The text isn't vertically centered in the box.

Please use labels and text to provide additional information.
Two screenshots are attached.

Thanks.
Mehmet
actual.png
34.4 KB View Download
expected.png
34.3 KB View Download

Comment 1 by tapted@chromium.org, Dec 12 2017

Cc: bettes@chromium.org
Owner: bsep@chromium.org
Status: WontFix (was: Untriaged)
This all looks correct and according to the spec. I don't think we want to change it.

UI review for this dialog is at https://bugs.chromium.org/p/chromium/issues/detail?id=726187&desc=2#c32

The bottom padding wants to match the side padding (16px) - that's correct.

This actually comes from 

  // By default, assume the footnote only contains text.
  frame->set_footnote_margins(
      LayoutProvider::Get()->GetDialogInsetsForContentType(TEXT, TEXT));

so DISTANCE_DIALOG_CONTENT_MARGIN_TOP_TEXT  DISTANCE_DIALOG_CONTENT_MARGIN_BOTTOM_TEXT 

    case views::DISTANCE_DIALOG_CONTENT_MARGIN_BOTTOM_CONTROL:
      return kHarmonyLayoutUnit * 3 / 2;
    case views::DISTANCE_DIALOG_CONTENT_MARGIN_BOTTOM_TEXT: {
      // This is reduced so there is about the same amount of visible
      // whitespace, compensating for the text's internal leading.
      return GetDistanceMetric(
                 views::DISTANCE_DIALOG_CONTENT_MARGIN_BOTTOM_CONTROL) -
             8;
    }
    case views::DISTANCE_DIALOG_CONTENT_MARGIN_TOP_CONTROL:
      return kHarmonyLayoutUnit;
    case views::DISTANCE_DIALOG_CONTENT_MARGIN_TOP_TEXT: {
      // See the comment in DISTANCE_DIALOG_CONTENT_MARGIN_BOTTOM_TEXT above.
      return GetDistanceMetric(
                 views::DISTANCE_DIALOG_CONTENT_MARGIN_TOP_CONTROL) -
             8;
    }

kHarmonyLayoutUnit = 16

16*3/2 - 8 = 16 above
16 - 8 = 8 below

Comment 2 by tapted@chromium.org, Dec 12 2017 

that is...

16*3/2 - 8 = 16 bottom
16 - 8 = 8 top

Comment 3 by bettes@chromium.org, Dec 12 2017 

Vertically centering (16 top and bottom) would be preferred

Comment 4 by bsep@chromium.org, Dec 12 2017

Status: Assigned (was: WontFix)
Yeah, the variable margins are just for the main content. I've been using INSETS_DIALOG_SUBSECTION for things like the footnote.
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 9 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fd34f035c9910af485f25a2056b6bc54f4bfa864

commit fd34f035c9910af485f25a2056b6bc54f4bfa864
Author: Bret Sepulveda <bsep@chromium.org>
Date: Tue Jan 09 01:51:54 2018

Fix bubble footnotes not being centered vertically.

Bug:  794196 
Change-Id: I6568e225965500b34de80382a5bb62ee703aa172
Reviewed-on: https://chromium-review.googlesource.com/855200
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527862}
[modify] https://crrev.com/fd34f035c9910af485f25a2056b6bc54f4bfa864/ui/views/bubble/bubble_dialog_delegate.cc

Comment 6 by bsep@chromium.org, Jan 9 2018

Status: Fixed (was: Assigned)

Comment 7 by sandeepkumars@chromium.org, Jan 10 2018

Labels: TE-Verified-M65 TE-Verified-65.0.3317.0
Tested the issue using #65.0.3317.0 on Mac 10.12.6 as per the steps mentioned in comment #0. Observed the text aligned vertically centered in the box now.

Please find the below screenshots, hence adding Verified labels.

Thanks!!
65.0.3317.0.png
140 KB View Download
65.0.3292.0.png
145 KB View Download
►

Sign in to add a comment