The alignment of chrome version number content/text description should be proper. (issue 893963004 by xdai@chromium.org)

9 views
Skip to first unread message

xd...@chromium.org

unread,
Feb 5, 2015, 8:16:23 PM2/5/15
to xiy...@chromium.org, chromium...@chromium.org, arv+...@chromium.org
Reviewers: xiyuan,

Message:
xiyuan@, I'm not sure if this issue need to be fixed, could you please help
to
take a look? Thanks!

Description:
The alignment of chrome version number content/text description should be
proper.

BUG=455084

Please review this at https://codereview.chromium.org/893963004/

Base URL: https://chromium.googlesource.com/chromium/src.git@master

Affected files (+1, -0 lines):
M chrome/browser/resources/about_version.css


Index: chrome/browser/resources/about_version.css
diff --git a/chrome/browser/resources/about_version.css
b/chrome/browser/resources/about_version.css
index
4f19be804539029281c98ac287a05f61ee35de8e..343dedfafa7cc7e0030852538b28b0060622af08
100644
--- a/chrome/browser/resources/about_version.css
+++ b/chrome/browser/resources/about_version.css
@@ -59,4 +59,5 @@ body {
font-family: monospace;
max-width: 430px;
padding-left: 5px;
+ vertical-align: bottom;
}


xiy...@chromium.org

unread,
Feb 5, 2015, 9:21:58 PM2/5/15
to xd...@chromium.org, chromium...@chromium.org, arv+...@chromium.org
Could you attach a screenshot of before and after applying the patch?

"vertical-align: bottom" makes the content table cell aligns to its bottom.
Would the multiple lines rows (e.g. Revisions, User Agent) all looks as
expected, especially the bottom of first line of the content compared with
the
bolded label.

If they look okay, then we are good. Otherwise, maybe we should consider
setting
a proper font-size for .label class instead of applying
vertical-align:bottom.

https://codereview.chromium.org/893963004/

xiy...@chromium.org

unread,
Feb 6, 2015, 3:15:53 PM2/6/15
to xd...@chromium.org, chromium...@chromium.org, arv+...@chromium.org
LGTM

Thanks for the screenshots.

https://codereview.chromium.org/893963004/

commi...@chromium.org

unread,
Feb 6, 2015, 4:07:18 PM2/6/15
to xd...@chromium.org, xiy...@chromium.org, chromium...@chromium.org, arv+...@chromium.org
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/893963004/1


https://codereview.chromium.org/893963004/

commi...@chromium.org

unread,
Feb 6, 2015, 5:13:21 PM2/6/15
to xd...@chromium.org, xiy...@chromium.org, chromium...@chromium.org, arv+...@chromium.org
Committed patchset #1 (id:1)

https://codereview.chromium.org/893963004/

commi...@chromium.org

unread,
Feb 6, 2015, 5:13:54 PM2/6/15
to xd...@chromium.org, xiy...@chromium.org, chromium...@chromium.org, arv+...@chromium.org
Patchset 1 (id:??) landed as
https://crrev.com/8b6f2beb570e0b476b87ae7c84f2e773a1f15d4b
Cr-Commit-Position: refs/heads/master@{#315121}

https://codereview.chromium.org/893963004/
Reply all
Reply to author
Forward
0 new messages