Implement Harmony typography specs |
||||||||||||||||||||||||||||||||||
Issue descriptionSpec: https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Spec#%2FSPEC-secondary-UI-01-typography.png%3Fz=width Specifically: * Provide named identifiers in the code that match the spec, i.e. "title", "body 2", etc. These may be in font-related code, but more likely will be specified on Label; see next item. * Ensure these result in fonts/labels that exactly match the spec. The amount of leading may be outside the control of just the FontList and require the containing Label to specify padding; similarly, if we specify colors via state identifiers like "primary"/"secondary", those would presumably be on the Label, not the FontList. However, where it makes sense, FontList should expose similar identifiers, so that if we need to render text directly (and not through a Label) the code is sane. * If possible, we should implement these identifiers the way other metrics in Harmony have been being implemented: with equivalent values for the pre-Harmony world, so that we can then switch UI code unconditionally to using these without (significantly) changing the existing behavior. I don't think our font sizes are exactly correct in the current world, and where they are I think people have had to do things like manually request size adjustments in Harmony-specific code (see e.g. https://cs.chromium.org/chromium/src/chrome/browser/ui/views/content_setting_bubble_contents.cc?type=cs&l=236 ). Instead a caller here should just need to do something like construct a Label with the "Title" style and get the font face, padding, and color for free. tapted@ has TODOs in the font code around moving away from the existing style enums (and, indeed, if we implement something like this, it should probably replace that system; not sure the right way to replace it is with sizing offsets in the callers though), so initially assigning to him, but +CC bsep since he also may be looking at this. You guys can work out the division of labor here, if tapted is overloaded.
,
Feb 16 2017
Are there places outside tests where it's legitimate to Not Care? Wondering if we can get away with forcing Label to require this anyway (and just pass FOR_TESTS in from tests), or providing a ...ForTesting() sort of factory function that tests can use if they don't want to specify this (I guess that's not less verbose). For TextContext... I'm torn. I had been thinking of providing enum values that matched the specs (headline, title, body 1, etc.), which allows the clearest translation of text to code and avoids redundant enum values or conflating layout concepts like "dialog" with text rendering stuff. But there is some merit to the route you've gone, where these specify more "where is this used" than "what is it". It might make maintenance in the future easier if we wanted to, say, change the button text font without touching any of the other things -- we'd change the mapping underneath this class instead of having to add/modify the enums and the callers who use them both. I'm still leaning toward the lower-level definition, but I'm not sure.
,
Feb 24 2017
> Are there places outside tests where it's legitimate to Not Care? I'm leaning towards 'no'. One rationale for allowing a default would be an attempt to retain some sanity during the epic refactor, but we should be able to drop it at some point. Maybe there are some cases around theming where the style isn't known when constructing the View hierarchy, but is set when the NativeTheme is established upon being inserted into the Widget. But moving away from gfx::FontList to something more descriptive like TextContext should remove that element anyway.
,
Mar 7 2017
,
Mar 9 2017
"Investigations" have started, and it's a little more dire than I had anticipated - https://codereview.chromium.org/2734113006/ "Step Zero" is just figuring out how to query the ResourceBundle to get a font of size "12" in a "default configuration". But (at least) on Windows, our bots disagree on what the "default" font size should be :/. We may need to smoke those out somehow. Sometimes it is 12pt, sometimes it is 11pt. And there will be differences between OS versions (e.g. macOS 10.9 and 10.12 disagree on some things). Getting full coverage is tricky. For "Leading", the font system provides Font::GetHeight() (different from GetFontSize()) which is essentially Leading. Specifically, it is the minimum amount of spacing that our RenderTextFoo implementations will put between baselines. Examples of "default" Leading: = Mac = 12pt font gets 15px leading "+3" 13pt font gets 16px leading "+3" 15pt font gets 19px leading "+4" 20pt font gets 25px leading "+5" = Win = 12pt font gets 15px leading "+3" 13pt font gets 17px leading "+4" 15pt font gets 20px leading "+2" 20pt font gets 28px leading "+8" = Linux = 12pt font gets 15px leading "+3" 13pt font gets 17px leading "+4" 15pt font gets 18px leading "+3" 20pt font gets 24px leading "+4" Of course the spec wants different leading, and a "12pt" font may be reconfigured by the user to something else. One possible plan, "Option 1", for leading is to take the above deltas as "truth", and then just bump up the *deltas* to get the leading we use in a multiline views::Label. = Mac = Body2 (12). Leading = GetHeight() + 5. (i.e. 20 in the "default" case) Body1 (13). Leading = GetHeight() + 4. (i.e. 20) Title (15). Leading = GetHeight() + 3. (i.e. 22) = Win = Body2 (12). Leading = GetHeight() + 5. (i.e. 20) Body1 (13). Leading = GetHeight() + 3. (i.e. 20) Title (15). Leading = GetHeight() + 2. (i.e. 22) = Linux = Body2 (12). Leading = GetHeight() + 5. (i.e. 20) Body1 (13). Leading = GetHeight() + 3. (i.e. 20) Title (15). Leading = GetHeight() + 4. (i.e. 22) Another, "Option 2", is just to ignore GetHeight() completely, but this might be ignoring some important properties of the particular font style. That is, = *ALL Platforms* = Body2 (12). Leading = GetFontSize() + 8. (i.e. 20) Body1 (13). Leading = GetFontSize() + 7. (i.e. 20) Title (15). Leading = GetFontSize() + 7. (i.e. 22) An orthogonal complication is that for some font systems, and for some layouts, the leading is not an integer, but really some floating point number that gets subjected to rounding. See comments in PlatformFontMac about this: https://cs.chromium.org/chromium/src/ui/gfx/platform_font_mac.mm?q=defaultLineHeightForFont+package:%5Echromium$&dr=C&l=212 (confusingly Mac's NSFont has a "leading" property, which is added to the font's ascent/descent. i.e. leading is the *extra* space added to the glyphs that determine line height, rather then the entire line height). Anyway, we'll probably need to iterate on this after seeing actual results. Plan is to go for "Option 1" to start.
,
Mar 14 2017
,
Mar 14 2017
,
Mar 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9577332120682b5400df78b16edcc0c759bb5881 commit 9577332120682b5400df78b16edcc0c759bb5881 Author: tapted <tapted@chromium.org> Date: Tue Mar 21 00:51:56 2017 "Bootstrap" a toolkit-views Typography spec. Under Harmony, we want to abstract away decisions about font selection. Harmony has a set of styles as part of the spec that encapsulate: - Font (typeface) - Font size, weight and color - Line spacing ("leading") gfx::Font[List] can represent Font, size and weight, but not color or line spacing. Also callers typically _never_ care about typeface, but APIs using gfx::Font are unable to encapsulate this. One side-effect is that a lot of callers use gfx::Font::Derive(..) rather than ResourceBundle::GetFontWithDelta(..) and so miss out on caching benefits. The typography concepts are split into "TextContext" and "TextStyle". The former describes the location of the text, and the latter describes its current state (e.g. Disabled) or other appearance (e.g. a Hyperlink) in that context. This CL exposes a set of abstract typography concepts at views::style, and explores the impact on client code by refactoring the views::Label constructor that takes a gfx::FontList to take a Label::CustomFont instead. Where it makes sense, callers are migrated to typography concepts rather than use Label::CustomFont. The transformations typically look like: - ui::ResourceBundle::MediumFont -> views::style::CONTEXT_DIALOG_TITLE "15pt" - GetFontListWithDelta(ui::kTitleFontSizeDelta) -> views::style::CONTEXT_DIALOG_TITLE - GetFontListWithDelta(1) -> (chrome) CONTEXT_BODY_TEXT_LARGE "13pt" - [default constructor] or ResourceBundle::BaseFont -> CONTEXT_BODY_TEXT_LARGE or views::style::CONTEXT_LABEL "12pt" - ui::ResourceBundle::SmallFont -> (chrome) CONTEXT_DEPRECATED_SMALL (Harmony doesn't have an 11pt font in the spec) - ui::ResourceBundle::BoldFont -> (chrome) STYLE_EMPHASIZED (Harmony doesn't have a "bold" font in the spec) These transformations, and the default TypographyProvider, are chosen to effectively be a "no-op" so that no existing dialogs change font sizes or other properties. Follow-ups will: - Implement a Harmony TypographyProvider. - Require all Label constructors to take a typography concept or Label::CustomFont. - Remove Label::SetFontList, Label::Set*Color, etc. (use typography or merge into CustomFont). - Remove gfx::FontList from the API of other toolkit-views classes. - Audit remaining users of Label::CustomFont to see if they can be replaced by Typography concepts. - Remove ResourceBundle::FontStyle (SmallFont, etc.) - Remove ui/default_style.h (ui::kFooFontSizeDelta, etc.) BUG= 691891 , 701241 Review-Url: https://codereview.chromium.org/2734113006 Cr-Commit-Position: refs/heads/master@{#458261} [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/chromeos/display/touch_calibrator/touch_calibrator_view.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/first_run/try_chrome_dialog_view.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/BUILD.gn [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/accessibility/invert_bubble_view.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/apps/app_info_dialog/app_info_header_panel.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel_unittest.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/autofill/password_generation_popup_view_views.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/chrome_views_delegate.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/chrome_views_delegate.h [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/content_setting_bubble_contents.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/create_application_shortcut_view.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/crypto_module_password_dialog_view_unittest.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/first_run_bubble.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/frame/glass_browser_frame_view.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc [add] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/harmony/chrome_typography.cc [add] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/harmony/chrome_typography.h [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/harmony/layout_delegate.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/harmony/layout_delegate.h [add] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/harmony/layout_delegate_unittest.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/ime/ime_warning_bubble_view.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/location_bar/keyword_hint_view.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/location_bar/location_icon_view.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/page_info/website_settings_popup_view.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/passwords/credentials_item_view.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/subtle_notification_view.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/sync/bubble_sync_promo_view_unittest.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/validation_message_bubble_view.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/test/BUILD.gn [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/app_list/views/speech_view.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/chromeos/ime/infolist_window.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/gfx/render_text_unittest.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/message_center/views/notifier_settings_view.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/BUILD.gn [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/bubble/bubble_frame_view.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/controls/button/label_button.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/controls/label.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/controls/label.h [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/controls/styled_label_unittest.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/controls/tabbed_pane/tabbed_pane.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/controls/textfield/textfield.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/mus/aura_init.cc [add] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/style/typography.cc [add] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/style/typography.h [add] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/style/typography_provider.cc [add] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/style/typography_provider.h [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/test/test_views_delegate.h [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/test/test_views_delegate_aura.cc [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/test/test_views_delegate_mac.mm [modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/views_delegate.h
,
Mar 23 2017
,
Apr 3 2017
mac lineheight sample screengrabs for https://codereview.chromium.org/2765883004/
,
Apr 3 2017
Same screengrabs, from Windows (10)
,
Apr 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/620e80b9d16fa22fb95269718c23e7f632cf9b76 commit 620e80b9d16fa22fb95269718c23e7f632cf9b76 Author: tapted <tapted@chromium.org> Date: Thu Apr 06 23:24:07 2017 Implement Harmony typography spec. Currently does font size and line height. Color is next. And there is still plumbing required to ensure more controls/dialogs have access to views::style (and stop using SetFontList). This CL establishes a core requirement to let us iterate on layout for specific dialogs. Example screenshots at https://crbug.com/691891#c10 BUG= 691891 Review-Url: https://codereview.chromium.org/2765883004 Cr-Commit-Position: refs/heads/master@{#462676} [modify] https://crrev.com/620e80b9d16fa22fb95269718c23e7f632cf9b76/chrome/browser/ui/BUILD.gn [modify] https://crrev.com/620e80b9d16fa22fb95269718c23e7f632cf9b76/chrome/browser/ui/views/harmony/harmony_layout_delegate.cc [modify] https://crrev.com/620e80b9d16fa22fb95269718c23e7f632cf9b76/chrome/browser/ui/views/harmony/harmony_layout_delegate.h [add] https://crrev.com/620e80b9d16fa22fb95269718c23e7f632cf9b76/chrome/browser/ui/views/harmony/harmony_typography_provider.cc [add] https://crrev.com/620e80b9d16fa22fb95269718c23e7f632cf9b76/chrome/browser/ui/views/harmony/harmony_typography_provider.h [modify] https://crrev.com/620e80b9d16fa22fb95269718c23e7f632cf9b76/chrome/browser/ui/views/harmony/layout_delegate_unittest.cc [modify] https://crrev.com/620e80b9d16fa22fb95269718c23e7f632cf9b76/ui/gfx/platform_font.h [modify] https://crrev.com/620e80b9d16fa22fb95269718c23e7f632cf9b76/ui/views/controls/button/label_button.cc [modify] https://crrev.com/620e80b9d16fa22fb95269718c23e7f632cf9b76/ui/views/controls/label.cc
,
Apr 7 2017
,
Apr 21 2017
tapted@ : I'm afraid Harmony typography spec does not take into account different needs of languages written in scripts other than Latin-Greek-Cyrillic. (c.f. Material Design spec has some consideration for these. : https://material.io/guidelines/style/typography.html ) 1) Roboto only covers Latin-Greek-Cyrillic. So, for other scripts, Noto Sans <Script_name> UI has to be used 2) It'd be ok if there's ample space below and above already specified in Harmony. If not, some scripts ("tall" scripts) need more line-spacing than LGC. ( Noto Sans <script> UI is designed to work within Roboto's vertical metrics. So, at least there'll be no clipping unless line-space is set to be smaller than Roboto's default line-spacing value. Recently, it's found that some Android apps do that leading to clipping. If Harmony has such a small line-spacing for some UI elements, it needs to be adjusted for tall scripts). 3) Some scripts are rather "dense" (e.g. Chinese/Japanese. Korean is on the border line) and too small a font size wouldn't work very well. Sorry that this bug is not likely to be the best place for raising this issue. (I couldn't find any reference to Harmony in MLs I'm a member of). Anyway, it'd be great if you could point me where I can raise this issue. Thank you,
,
Apr 22 2017
Thanks for raising this. Although the Harmony spec describes everything in point sizes, we haven't deviated from what Chrome currently does when dealing with non-Latin scripts for font size (everything is still a "delta" - point sizes are just the "target" size for a default US/Latin setup). Line spacing is new to Chrome UI, and the approach described in #c5 and implemented in r462676 has some consideration for the issues you raise. Effectively the implementation tries to add the same _additional_ line spacing that would be added in the Latin script for a default OS configuration, to the locale-sensitive system FontList. We only add spacing -- Chrome's UI text rendering doesn't allow a line of text to have line spacing shorter than the tallest glyph on that line. Generally, 2-3 pixels are added above and below the the maximum glyph height reported in the locale-sensitive system font to achieve the look that the Harmony spec describes. We'll be putting dialogs through thorough review across languages and system font settings (e.g. bold/larger fonts), and can adjust things if a locale looks "off". Let us know if there are specific/tricky locales you know of that we should focus on, or if you see any problems with chrome://flags/#secondary-ui-md flipped.
,
May 5 2017
In https://codereview.chromium.org/2801583002/ I'm exploring what would happen if we interpret font weights from the spec as a delta. On Windows, the UI font can be set BOLD, which is 3 notches bolder than NORMAL. So BOLD becomes BLACK (the heaviest weight) and MEDIUM becomes EXTRA_BOLD (one below that). On Windows, MD buttons are specified as BOLD (MEDIUM elsewhere). Attaching some screenshots for how this would look.
,
May 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e3880da51bf9da14f748d00ac7b5741989107696 commit e3880da51bf9da14f748d00ac7b5741989107696 Author: tapted <tapted@chromium.org> Date: Tue May 16 05:35:17 2017 Use views::style for buttons, bootstrap ash_typography to do so. This makes LabelButton::AdjustFontSize() obsolete. LabelButton::SetFontSize() is also mostly obsolete. Remove it. Two remaining cases are button subclasses that are never "default" buttons so they can just set the font list on the button->label() directly. BUG= 691891 , 623987 TEST=LabelButtonTest.TextSizeFromContext (replaces AdjustFontSize) Review-Url: https://codereview.chromium.org/2801583002 Cr-Commit-Position: refs/heads/master@{#472019} [modify] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/ash/public/cpp/BUILD.gn [add] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/ash/public/cpp/ash_typography.cc [add] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/ash/public/cpp/ash_typography.h [modify] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/ash/system/session/logout_button_tray.cc [modify] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/ash/system/toast/toast_overlay.cc [modify] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/chrome/browser/ui/views/harmony/chrome_typography.cc [modify] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/chrome/browser/ui/views/harmony/chrome_typography.h [modify] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/chrome/browser/ui/views/harmony/harmony_typography_provider.cc [modify] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/chrome/browser/ui/views/harmony/layout_provider_unittest.cc [modify] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/chrome/browser/ui/views/profiles/avatar_button.cc [modify] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/chrome/browser/ui/views/toolbar/app_menu.cc [modify] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/ui/views/controls/button/label_button.cc [modify] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/ui/views/controls/button/label_button.h [modify] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/ui/views/controls/button/label_button_unittest.cc [modify] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/ui/views/controls/button/md_text_button.cc [modify] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/ui/views/controls/button/md_text_button.h [modify] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/ui/views/style/typography.h [modify] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/ui/views/style/typography_provider.cc [modify] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/ui/views/style/typography_provider.h [modify] https://crrev.com/e3880da51bf9da14f748d00ac7b5741989107696/ui/views/touchui/touch_selection_menu_runner_views.cc
,
May 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bc62dfeaf37c83a79f393cb536fb22b39ea87097 commit bc62dfeaf37c83a79f393cb536fb22b39ea87097 Author: tapted <tapted@chromium.org> Date: Sun May 21 03:10:15 2017 Support finer grained font weights on Mac. We want to explore a MEDIUM weight for button text under Harmony. This is consistent with MD and helps compensate for buttons not using solid black for the font color. The San Francisco font in 10.11 and 10.12 has good support for finer grained weights and is the font Chrome UI is mostly concerned with, so focus testing on that but survey expectations for all supported OS versions. BUG= 691891 Review-Url: https://codereview.chromium.org/2869803005 Cr-Commit-Position: refs/heads/master@{#473457} [modify] https://crrev.com/bc62dfeaf37c83a79f393cb536fb22b39ea87097/ui/gfx/platform_font_mac.mm [modify] https://crrev.com/bc62dfeaf37c83a79f393cb536fb22b39ea87097/ui/gfx/platform_font_mac_unittest.mm [modify] https://crrev.com/bc62dfeaf37c83a79f393cb536fb22b39ea87097/ui/views/examples/example_combobox_model.cc [modify] https://crrev.com/bc62dfeaf37c83a79f393cb536fb22b39ea87097/ui/views/examples/example_combobox_model.h [modify] https://crrev.com/bc62dfeaf37c83a79f393cb536fb22b39ea87097/ui/views/examples/text_example.cc [modify] https://crrev.com/bc62dfeaf37c83a79f393cb536fb22b39ea87097/ui/views/examples/text_example.h
,
May 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cb23bc6b0dae9c82bb9845b4ea367d72e8c2edaf commit cb23bc6b0dae9c82bb9845b4ea367d72e8c2edaf Author: tapted <tapted@chromium.org> Date: Sun May 21 04:38:35 2017 Revert of Support finer grained font weights on Mac. (patchset #6 id:240001 of https://codereview.chromium.org/2869803005/ ) Reason for revert: Need to test 10.11 again. failures in the build cycle at https://luci-milo.appspot.com/buildbot/chromium.mac/Mac10.11%20Tests/12666 Original issue's description: > Support finer grained font weights on Mac. > > We want to explore a MEDIUM weight for button text under Harmony. This > is consistent with MD and helps compensate for buttons not using solid > black for the font color. > > The San Francisco font in 10.11 and 10.12 has good support for finer > grained weights and is the font Chrome UI is mostly concerned with, so > focus testing on that but survey expectations for all supported OS > versions. > > BUG= 691891 > > Review-Url: https://codereview.chromium.org/2869803005 > Cr-Commit-Position: refs/heads/master@{#473457} > Committed: https://chromium.googlesource.com/chromium/src/+/bc62dfeaf37c83a79f393cb536fb22b39ea87097 TBR=asvitkine@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 691891 Review-Url: https://codereview.chromium.org/2898613002 Cr-Commit-Position: refs/heads/master@{#473460} [modify] https://crrev.com/cb23bc6b0dae9c82bb9845b4ea367d72e8c2edaf/ui/gfx/platform_font_mac.mm [modify] https://crrev.com/cb23bc6b0dae9c82bb9845b4ea367d72e8c2edaf/ui/gfx/platform_font_mac_unittest.mm [modify] https://crrev.com/cb23bc6b0dae9c82bb9845b4ea367d72e8c2edaf/ui/views/examples/example_combobox_model.cc [modify] https://crrev.com/cb23bc6b0dae9c82bb9845b4ea367d72e8c2edaf/ui/views/examples/example_combobox_model.h [modify] https://crrev.com/cb23bc6b0dae9c82bb9845b4ea367d72e8c2edaf/ui/views/examples/text_example.cc [modify] https://crrev.com/cb23bc6b0dae9c82bb9845b4ea367d72e8c2edaf/ui/views/examples/text_example.h
,
May 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/124eb0b5af215ce2df373be6552228d79090256e commit 124eb0b5af215ce2df373be6552228d79090256e Author: tapted <tapted@chromium.org> Date: Mon May 22 01:54:14 2017 Support finer grained font weights on Mac. We want to explore a MEDIUM weight for button text under Harmony. This is consistent with MD and helps compensate for buttons not using solid black for the font color. The San Francisco font in 10.12 has good support for finer grained weights and is the font Chrome UI is mostly concerned with, so focus testing on that but survey expectations for all supported OS versions. BUG= 691891 Review-Url: https://codereview.chromium.org/2869803005 Cr-Commit-Position: refs/heads/master@{#473490} [modify] https://crrev.com/124eb0b5af215ce2df373be6552228d79090256e/ui/gfx/platform_font_mac.mm [modify] https://crrev.com/124eb0b5af215ce2df373be6552228d79090256e/ui/gfx/platform_font_mac_unittest.mm [modify] https://crrev.com/124eb0b5af215ce2df373be6552228d79090256e/ui/views/examples/example_combobox_model.cc [modify] https://crrev.com/124eb0b5af215ce2df373be6552228d79090256e/ui/views/examples/example_combobox_model.h [modify] https://crrev.com/124eb0b5af215ce2df373be6552228d79090256e/ui/views/examples/text_example.cc [modify] https://crrev.com/124eb0b5af215ce2df373be6552228d79090256e/ui/views/examples/text_example.h
,
May 30 2017
,
Jun 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/30fe63c7bb9623a59ca6346ae956f5290f0864be commit 30fe63c7bb9623a59ca6346ae956f5290f0864be Author: tapted <tapted@chromium.org> Date: Sat Jun 03 12:04:04 2017 Remove views::Label::SetDisabledColor(). Replace with typography colors. Everything calling Label::SetDisabledColor() is just using it as a roundabout way to make the text grey instead of black. This is bad because "disabled" is a property that is fed through to a11y clients. Alternatively, a consumer calls SetDisabledColor(), but the Label is never actually disabled. In Harmony, the typography spec distinguishes between "secondary" grey and "hint" grey which is what consumers should actually use to describe their text (once there's a mock which says which should be used). Bootstrap views::style::GetColor() to support consumers that want a grey Label to specify style::STYLE_DISABLED, or STYLE_HINT when creating a Label. Only LabelButton was using SetDisabledColor() properly. It sometimes gets a disabled color from the NativeTheme and sometimes overrides it. Nothing else needs this, so move disabled-label functionality to a Label subclass in a follow-up: crrev.com/2913933002 BUG= 691891 Review-Url: https://codereview.chromium.org/2910153002 Cr-Commit-Position: refs/heads/master@{#476897} [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/ash/accelerators/exit_warning_handler.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/ash/sticky_keys/sticky_keys_overlay.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/ash/system/toast/toast_overlay.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/chrome/browser/chromeos/display/touch_calibrator/touch_calibrator_view.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/chrome/browser/chromeos/ui/idle_app_name_notification_view.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/chrome/browser/chromeos/ui/kiosk_external_update_notification.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/chrome/browser/ui/views/harmony/chrome_typography.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/chrome/browser/ui/views/harmony/chrome_typography.h [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/chrome/browser/ui/views/harmony/harmony_typography_provider.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/chrome/browser/ui/views/harmony/harmony_typography_provider.h [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/chrome/browser/ui/views/payments/editor_view_controller.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/chrome/browser/ui/views/payments/order_summary_view_controller.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/chrome/browser/ui/views/payments/payment_request_views_util.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/chrome/browser/ui/views/payments/payment_request_views_util.h [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/chrome/browser/ui/views/payments/payment_sheet_view_controller.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/chrome/browser/ui/views/profiles/profile_chooser_view.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/chrome/browser/ui/views/sad_tab_view.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/ui/chromeos/ime/candidate_view.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/ui/views/controls/button/label_button.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/ui/views/controls/label.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/ui/views/controls/label.h [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/ui/views/controls/link.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/ui/views/controls/link.h [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/ui/views/style/typography.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/ui/views/style/typography.h [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/ui/views/style/typography_provider.cc [modify] https://crrev.com/30fe63c7bb9623a59ca6346ae956f5290f0864be/ui/views/style/typography_provider.h
,
Jun 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/115f6f8b1cb43f0c2d8a32ff54774a6ff3212eb3 commit 115f6f8b1cb43f0c2d8a32ff54774a6ff3212eb3 Author: tapted <tapted@chromium.org> Date: Mon Jun 05 03:11:22 2017 Remove text_ prefixes from the views::style::Get* helper args. This makes it consistent with the TypographyProvider interface. BUG= 691891 Review-Url: https://codereview.chromium.org/2918003002 Cr-Commit-Position: refs/heads/master@{#476932} [modify] https://crrev.com/115f6f8b1cb43f0c2d8a32ff54774a6ff3212eb3/ui/views/style/typography.cc [modify] https://crrev.com/115f6f8b1cb43f0c2d8a32ff54774a6ff3212eb3/ui/views/style/typography.h
,
Jun 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e699d8e27d888b832000c598fa10e8788d1042ef commit e699d8e27d888b832000c598fa10e8788d1042ef Author: tapted <tapted@chromium.org> Date: Thu Jun 22 00:28:15 2017 Move views::Label DisabledColor logic into views::LabelButtonLabel Only views::LabelButton uses disabled colors on a views::Label in a meaningful way. Move the logic into a subclass. BUG= 691891 Review-Url: https://codereview.chromium.org/2913933002 Cr-Commit-Position: refs/heads/master@{#481374} [modify] https://crrev.com/e699d8e27d888b832000c598fa10e8788d1042ef/ui/views/BUILD.gn [modify] https://crrev.com/e699d8e27d888b832000c598fa10e8788d1042ef/ui/views/controls/button/label_button.cc [modify] https://crrev.com/e699d8e27d888b832000c598fa10e8788d1042ef/ui/views/controls/button/label_button.h [add] https://crrev.com/e699d8e27d888b832000c598fa10e8788d1042ef/ui/views/controls/button/label_button_label.cc [add] https://crrev.com/e699d8e27d888b832000c598fa10e8788d1042ef/ui/views/controls/button/label_button_label.h [add] https://crrev.com/e699d8e27d888b832000c598fa10e8788d1042ef/ui/views/controls/button/label_button_label_unittest.cc [modify] https://crrev.com/e699d8e27d888b832000c598fa10e8788d1042ef/ui/views/controls/label.cc [modify] https://crrev.com/e699d8e27d888b832000c598fa10e8788d1042ef/ui/views/controls/label.h
,
Jun 28 2017
,
Jun 28 2017
Updated description with the new spec, just for buttons, from https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Spec#%2FSPEC-secondary-UI-03-buttons.png%3Fz=width file: 2017-06-09-SPEC-secondary-UI-03-buttons.png Button colors in other spec png files should be ignored - those spec sheets haven't been updated yet.
,
Jun 28 2017
,
Jun 28 2017
new button color screengrabs. Note enabled --> (255-117)/255 = .54117 (0.54a) disabled -> (255-159)/255 = .37647 (0.38a) Cl -> https://codereview.chromium.org/2957263002
,
Jun 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2d322753596b18ed0d10242424edcefe4da835f2 commit 2d322753596b18ed0d10242424edcefe4da835f2 Author: tapted <tapted@chromium.org> Date: Fri Jun 30 07:17:59 2017 Use kColorId_LabelEnabledColor in Checkbox, not kColorId_ButtonEnabledColor Checkbox uses views::Button::STYLE_TEXTBUTTON, but only STYLE_BUTTON uses kColorId_ButtonEnabledColor in views::LabelButton (which views::Checkbox inherits from). Fix it. Also use the CreateDefaultInkDropRipple helper in Checkbox::CreateInkDropRipple() Ripples under harmony aren't derived from text colors, so we want to consolidate calls to the SquareInkDropRipple constructor. This special case isn't actually special. BUG= 691891 Review-Url: https://codereview.chromium.org/2963083003 Cr-Commit-Position: refs/heads/master@{#483650} [modify] https://crrev.com/2d322753596b18ed0d10242424edcefe4da835f2/ui/views/controls/button/checkbox.cc
,
Jun 30 2017
Note for possible future work: as I was doing https://chromium-review.googlesource.com/c/557139, I noticed there's no mechanisms for StyledLabels to set their text color given a context/style like normal Labels have. I worked around it for now by giving AutoSigninFirstRunDialogView an override of OnNativeThemeChanged, which can be removed if we update StyledLabel.
,
Jul 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d007b3e7c64c2fdaf91c70ed2bc89205fcdfcb41 commit d007b3e7c64c2fdaf91c70ed2bc89205fcdfcb41 Author: tapted <tapted@chromium.org> Date: Mon Jul 03 06:37:24 2017 Update dialog button text color for Harmony per the latest button spec Only use the new colors when using HarmonyTypographyProvider. Diverge from NativeTheme for this since there's too much indirection. `kButtonEnabledColor` is gfx::kChromeIconGrey, but kButtonEnabledColor is used for lots of other things. Note we still use NativeTheme when the Harmony spec is ignored (e.g. OS themes with inverted or high-constrast themes). This CL refactors the code in HarmonyTypographyProvider to be more explicit about when the Harmony spec needs to be ignored. BUG= 691891 , 738292 Review-Url: https://codereview.chromium.org/2957263002 Cr-Commit-Position: refs/heads/master@{#483954} [modify] https://crrev.com/d007b3e7c64c2fdaf91c70ed2bc89205fcdfcb41/chrome/browser/ui/views/harmony/harmony_typography_provider.cc [modify] https://crrev.com/d007b3e7c64c2fdaf91c70ed2bc89205fcdfcb41/ui/views/controls/button/md_text_button.cc [modify] https://crrev.com/d007b3e7c64c2fdaf91c70ed2bc89205fcdfcb41/ui/views/controls/label.h [modify] https://crrev.com/d007b3e7c64c2fdaf91c70ed2bc89205fcdfcb41/ui/views/style/typography_provider.cc
,
Jul 3 2017
> Note for possible future work: as I was doing https://chromium-review.googlesource.com/c/557139, I noticed there's no mechanisms for StyledLabels to set their text color given a context/style like normal Labels have. I worked around it for now by giving AutoSigninFirstRunDialogView an override of OnNativeThemeChanged, which can be removed if we update StyledLabel. Yeah - StyledLabel should support using style constants from typography.h. It currently does not - it uses the views::Label constructor that I have a "TODO(me)" on. // Create Labels with style::CONTEXT_CONTROL_LABEL and style::STYLE_PRIMARY. // TODO(tapted): Remove these. Callers must specify a context or use the // constructor taking a CustomFont. Label(); explicit Label(const base::string16& text);
,
Jul 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1397566bcda02ab4f38737a2bd46a2e013341e0b commit 1397566bcda02ab4f38737a2bd46a2e013341e0b Author: tapted <tapted@chromium.org> Date: Mon Jul 03 09:19:28 2017 Use style::CONTEXT_TEXTFIELD for Combobox and Textfield Combobox currently uses all of the folowing text-color constants: ui::NativeTheme::kColorId_TextfieldDefaultColor ui::NativeTheme::kColorId_TextfieldReadOnlyColor ui::NativeTheme::kColorId_LabelEnabledColor ui::NativeTheme::kColorId_LabelDisabledColor ui::NativeTheme::kColorId_ButtonEnabledColor That's too many, and kColorId_ButtonEnabledColor wants to go lighter under Harmony. Stop using it for Combobox. Also Textfields under Harmony are currently using Black only, but they should use the "primary" text color, which is a tiny bit lighter than black. Consolidate them both under views::style::CONTEXT_TEXTFIELD and a corresponding disabled style. Some tests didn't have a ViewsDelegate. Fix that. BUG= 691891 Review-Url: https://codereview.chromium.org/2964453002 Cr-Commit-Position: refs/heads/master@{#483968} [modify] https://crrev.com/1397566bcda02ab4f38737a2bd46a2e013341e0b/chrome/browser/ui/views/omnibox/omnibox_view_views_unittest.cc [modify] https://crrev.com/1397566bcda02ab4f38737a2bd46a2e013341e0b/chrome/browser/ui/views/payments/validating_textfield_unittest.cc [modify] https://crrev.com/1397566bcda02ab4f38737a2bd46a2e013341e0b/ui/views/cocoa/bridged_native_widget_unittest.mm [modify] https://crrev.com/1397566bcda02ab4f38737a2bd46a2e013341e0b/ui/views/controls/combobox/combobox.cc [modify] https://crrev.com/1397566bcda02ab4f38737a2bd46a2e013341e0b/ui/views/controls/textfield/textfield.cc [modify] https://crrev.com/1397566bcda02ab4f38737a2bd46a2e013341e0b/ui/views/style/typography_provider.cc
,
Aug 2 2017
This is a pretty broad bug title, and I'm not totally clear where we stand. My impression is that most of the spec is basically working at this point. Should we split off known remaining issues to separate bugs and mark this fixed? Leaving as P1 until then.
,
Aug 2 2017
I should act on #c32.. (typography support for views::StyledLabel) otherwise the main thing that remains is just to implement SECONDARY/HINT styles in specific dialogs where appropriate. E.g. http-auth is done, but little else is using SECONDARY right now AFAIK. And then.. try to phase out Label::SetFontList, since that will make it impossible for new dialogs to use random sizes/styles -- only styles from views/style/typography.h, chrome_typography.h, or ash_typography.h will be allowed. Then we should kill the Label::CustomFont constructor. There's still RenderText::SetFontList() and the potential for a subclass to override Label::CreateRenderText() if something is truly unique. I think only the StyledLabel part is blocking. The rest can be follow-ups in other/new bugs.
,
Aug 16 2017
,
Sep 26 2017
The styled label piece of the puzzle is coming together in https://chromium-review.googlesource.com/c/chromium/src/+/649930 One current annoyance: there is exactly one place where something wants - a tooltip for a StyledLabel range, and - that same range to be underlined, and - it's only on ChromeOS. It's the "EchoDialogView", and I made http://crbug.com/768663 for it. The dialog is weird. If any UX folk want to weigh in on how it should eventually look, that could change my approach here.
,
Sep 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d0c7052ee977d4937edbbfe37498601ee0a802fb commit d0c7052ee977d4937edbbfe37498601ee0a802fb Author: Trent Apted <tapted@chromium.org> Date: Fri Sep 29 01:35:37 2017 Support typography styles in views::StyledLabel. Adds initial support to views::Link as well, by allowing it to pass TextContext to its parent views::Label constructor. The entire StyledLabel has a single TextContext. A default TextStyle can be optionally overridden on a RangeStyleInfo. Audited the StyledLabel consumers: One consumer (EchoDialogView) uses underlined text outside of a Link. The dialog is under review ( http://crbug.com/768663 ), so don't add a style for it. The PageInfoBubble was the only consumer that wanted an underline on its links. Remove underlines from all links in the page info bubble for consistency with the rest of Chrome's UI. All the (slow) FontList::Derive(..) calls disappear from StyledLabel. Most fonts just come from the ResourceBundle cache now, unless they need a custom_font (which is currently just EchoDialogView). Bug: 691891 Change-Id: I6cca4480d366d2578ac01da4d604a414415ffbc4 Reviewed-on: https://chromium-review.googlesource.com/649930 Commit-Queue: Trent Apted <tapted@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Bret Sepulveda <bsep@chromium.org> Cr-Commit-Position: refs/heads/master@{#505246} [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/chromeos/ui/echo_dialog_view.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/ui/views/autofill/password_generation_popup_view_views.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/ui/views/autofill/password_generation_popup_view_views.h [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/ui/views/desktop_ios_promotion/desktop_ios_promotion_footnote_view.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/ui/views/extensions/request_file_system_dialog_view.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/ui/views/page_info/page_info_bubble_view.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/ui/views/passwords/account_chooser_dialog_view.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/ui/views/passwords/auto_signin_first_run_dialog_view.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/ui/views/passwords/auto_signin_first_run_dialog_view.h [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/ui/views/payments/payment_sheet_view_controller.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/ui/views/platform_keys_certificate_selector_chromeos.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/ui/views/profiles/forced_reauthentication_dialog_view.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/ui/views/profiles/profile_chooser_view.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/ui/views/session_crashed_bubble_view.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/ui/views/settings_reset_prompt_dialog.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/ui/views/subtle_notification_view.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/ui/views/sync/bubble_sync_promo_view.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/chrome/browser/ui/views/sync/profile_signin_confirmation_dialog_views.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/ui/views/controls/link.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/ui/views/controls/link.h [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/ui/views/controls/message_box_view.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/ui/views/controls/styled_label.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/ui/views/controls/styled_label.h [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/ui/views/controls/styled_label_unittest.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/ui/views/test/test_layout_provider.cc [modify] https://crrev.com/d0c7052ee977d4937edbbfe37498601ee0a802fb/ui/views/test/test_layout_provider.h
,
Oct 26 2017
I said in #c35 this becomes a non-blocking issue once StyledLabel is done. Which it is! So I'll close this out. There may still be follow-ups. E.g. - Deprecate & Remove SetFontList methods from controls, migrate client code to typography - Remove CONTEXT_DEPRECATED_SMALL from chrome_typography.h
,
Nov 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5 commit 68bc0bac6c4c11bd1e13c5778f1eaea0196430d5 Author: Tomasz Moniuszko <tmoniuszko@opera.com> Date: Tue Nov 14 15:35:58 2017 Allow consulting ThemeProvider when asking TypographyProvider for color Any custom View may provide ThemeProvider. TypographyProvider implementation should have possibility to consult the ThemeProvider in similar way NativeTheme can be consulted. Bug: 691891 Change-Id: I4d67dfa870d9c439292f74c96e408bb7a9471059 Reviewed-on: https://chromium-review.googlesource.com/730583 Commit-Queue: Tomasz Moniuszko <tmoniuszko@opera.com> Reviewed-by: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#516308} [modify] https://crrev.com/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5/chrome/browser/ui/views/harmony/chrome_typography.cc [modify] https://crrev.com/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5/chrome/browser/ui/views/harmony/chrome_typography.h [modify] https://crrev.com/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5/chrome/browser/ui/views/harmony/harmony_typography_provider.cc [modify] https://crrev.com/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5/chrome/browser/ui/views/harmony/harmony_typography_provider.h [modify] https://crrev.com/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5/chrome/browser/ui/views/hover_button.cc [modify] https://crrev.com/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5/chrome/browser/ui/views/page_info/chosen_object_row.cc [modify] https://crrev.com/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5/chrome/browser/ui/views/page_info/page_info_bubble_view.cc [modify] https://crrev.com/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5/ui/chromeos/ime/candidate_view.cc [modify] https://crrev.com/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5/ui/views/controls/button/checkbox.cc [modify] https://crrev.com/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5/ui/views/controls/button/label_button_label.cc [modify] https://crrev.com/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5/ui/views/controls/button/md_text_button.cc [modify] https://crrev.com/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5/ui/views/controls/combobox/combobox.cc [modify] https://crrev.com/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5/ui/views/controls/label.cc [modify] https://crrev.com/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5/ui/views/controls/textfield/textfield.cc [modify] https://crrev.com/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5/ui/views/style/typography.cc [modify] https://crrev.com/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5/ui/views/style/typography.h [modify] https://crrev.com/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5/ui/views/style/typography_provider.cc [modify] https://crrev.com/68bc0bac6c4c11bd1e13c5778f1eaea0196430d5/ui/views/style/typography_provider.h |
||||||||||||||||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||||||||||||||||
/Secondary%20UI%20Previews%20and%20specs%20(exports)/Spec#%2FSPEC-secondary-UI-01-typography.png%3Fz=width){kind=link}
/Secondary%20UI%20Previews%20and%20specs%20(exports)/Spec#%2FSPEC-secondary-UI-03-buttons.png%3Fz=width){kind=link}
Comment 1 by tapted@chromium.org
, Feb 15 2017In my earlier adventures, the biggest annoyance seemed to come from code relying on "defaults", and this is still the case. E.g. views::Label constructed with just text needs to pick a font size somehow. One approach might be to force every object that ever takes some std::string16 to display at some point to also require an explicit, or abstract, font-description-of-some-kind along with that text (either provided with it, or set on the object earlier). However, I feel that this isn't realistic -- too much (e.g. tests) don't care. So I'm currently thinking something like: namespace views { enum class TextContext { // TypeContext? TypographyContext? TextLocation? ideas? DEFAULT, // DEFAULT_FOR_TESTING? .. Basically, nothing should pass this into ui/views, but ui/views will need it internally to avoid regressions. HEADLINE, DIALOG_TITLE, DIALOG_MESSAGE, // "Body 1" BUTTON_TEXT, CONTROL_LABEL, // "Body 2". (or OTHER_CONTROL_TEXT / DIALOG_LABEL -- maybe make multiple things that all use "Body 2"?) }; enum class TextStyle { DEFAULT, BODY, DISABLED, LINK, HINT, }; class TypographyProvider { public: virtual gfx::FontList GetFontList(TextContext context, TextStyle style) = 0; virtual SkColor GetFontColor(TextContext context, TextStyle style) = 0; virtual int GetLeading(TextContext context, TextStyle style) = 0 }; class ViewsDelegate { public: ... virtual TypographyProvider* GetTypeographyProvider() = 0; }; } // namespace views