Commit 620e80b9 authored by tapted's avatar tapted Committed by Commit bot

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}
parent cafbdba2
......@@ -1502,6 +1502,8 @@ split_static_library("ui") {
"views/harmony/chrome_typography.h",
"views/harmony/harmony_layout_delegate.cc",
"views/harmony/harmony_layout_delegate.h",
"views/harmony/harmony_typography_provider.cc",
"views/harmony/harmony_typography_provider.h",
"views/harmony/layout_delegate.cc",
"views/harmony/layout_delegate.h",
"views/location_bar/location_bar_bubble_delegate_view.cc",
......
......@@ -96,3 +96,8 @@ int HarmonyLayoutDelegate::GetDialogPreferredWidth(DialogWidth width) const {
NOTREACHED();
return 0;
}
const views::TypographyProvider& HarmonyLayoutDelegate::GetTypographyProvider()
const {
return typography_provider_;
}
......@@ -5,6 +5,7 @@
#ifndef CHROME_BROWSER_UI_VIEWS_HARMONY_HARMONY_LAYOUT_DELEGATE_H_
#define CHROME_BROWSER_UI_VIEWS_HARMONY_HARMONY_LAYOUT_DELEGATE_H_
#include "chrome/browser/ui/views/harmony/harmony_typography_provider.h"
#include "chrome/browser/ui/views/harmony/layout_delegate.h"
class HarmonyLayoutDelegate : public LayoutDelegate {
......@@ -25,8 +26,11 @@ class HarmonyLayoutDelegate : public LayoutDelegate {
bool IsHarmonyMode() const override;
int GetDialogPreferredWidth(DialogWidth width) const override;
bool ShouldShowWindowIcon() const override;
const views::TypographyProvider& GetTypographyProvider() const override;
private:
const HarmonyTypographyProvider typography_provider_;
DISALLOW_COPY_AND_ASSIGN(HarmonyLayoutDelegate);
};
......
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/ui/views/harmony/harmony_typography_provider.h"
#include "chrome/browser/ui/views/harmony/chrome_typography.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/gfx/platform_font.h"
const gfx::FontList& HarmonyTypographyProvider::GetFont(int text_context,
int text_style) const {
// "Target" font size constants from the Harmony spec.
constexpr int kHeadlineSize = 20;
constexpr int kTitleSize = 15;
constexpr int kBodyTextLargeSize = 13;
constexpr int kDefaultSize = 12;
#if defined(OS_WIN)
constexpr gfx::Font::Weight kButtonFontWeight = gfx::Font::Weight::BOLD;
#else
constexpr gfx::Font::Weight kButtonFontWeight = gfx::Font::Weight::MEDIUM;
#endif
int size_delta = kDefaultSize - gfx::PlatformFont::kDefaultBaseFontSize;
gfx::Font::Weight font_weight = gfx::Font::Weight::NORMAL;
switch (text_context) {
case CONTEXT_HEADLINE:
size_delta = kHeadlineSize - gfx::PlatformFont::kDefaultBaseFontSize;
break;
case views::style::CONTEXT_DIALOG_TITLE:
size_delta = kTitleSize - gfx::PlatformFont::kDefaultBaseFontSize;
break;
case CONTEXT_BODY_TEXT_LARGE:
size_delta = kBodyTextLargeSize - gfx::PlatformFont::kDefaultBaseFontSize;
break;
case views::style::CONTEXT_BUTTON:
font_weight = kButtonFontWeight;
break;
default:
break;
}
// Ignore |text_style| since it only affects color in the Harmony spec.
return ui::ResourceBundle::GetSharedInstance().GetFontListWithDelta(
size_delta, gfx::Font::NORMAL, font_weight);
}
SkColor HarmonyTypographyProvider::GetColor(int text_context,
int text_style) const {
// TODO(tapted): Look up colors from the spec.
return SK_ColorBLACK;
}
int HarmonyTypographyProvider::GetLineHeight(int text_context,
int text_style) const {
// "Target" line height constants from the Harmony spec. A default OS
// configuration should use these heights. However, if the user overrides OS
// defaults, then GetLineHeight() should return the height that would add the
// same extra space between lines as the default configuration would have.
constexpr int kHeadlineHeight = 32;
constexpr int kTitleHeight = 22;
constexpr int kBodyHeight = 20; // For both large and small.
// Button text should always use the minimum line height for a font to avoid
// unnecessarily influencing the height of a button.
constexpr int kButtonAbsoluteHeight = 0;
// The platform-specific heights (i.e. gfx::Font::GetHeight()) that result when
// asking for the target size constants in HarmonyTypographyProvider::GetFont()
// in a default OS configuration.
// TODO(tapted): Update these with constants specific to an OS point version.
#if defined(OS_MACOSX)
constexpr int kHeadlinePlatformHeight = 25;
constexpr int kTitlePlatformHeight = 19;
constexpr int kBodyTextLargePlatformHeight = 16;
constexpr int kBodyTextSmallPlatformHeight = 15;
#elif defined(OS_WIN)
constexpr int kHeadlinePlatformHeight = 28;
constexpr int kTitlePlatformHeight = 20;
constexpr int kBodyTextLargePlatformHeight = 17;
constexpr int kBodyTextSmallPlatformHeight = 15;
#else
constexpr int kHeadlinePlatformHeight = 24;
constexpr int kTitlePlatformHeight = 18;
constexpr int kBodyTextLargePlatformHeight = 17;
constexpr int kBodyTextSmallPlatformHeight = 15;
#endif
// The style of the system font used to determine line heights.
constexpr int kTemplateStyle = views::style::STYLE_PRIMARY;
// TODO(tapted): These statics should be cleared out when something invokes
// ResourceBundle::ReloadFonts(). Currently that only happens on ChromeOS.
// See http://crbug.com/708943.
static const int headline_height =
GetFont(CONTEXT_HEADLINE, kTemplateStyle).GetHeight() -
kHeadlinePlatformHeight + kHeadlineHeight;
static const int title_height =
GetFont(views::style::CONTEXT_DIALOG_TITLE, kTemplateStyle).GetHeight() -
kTitlePlatformHeight + kTitleHeight;
static const int body_large_height =
GetFont(CONTEXT_BODY_TEXT_LARGE, kTemplateStyle).GetHeight() -
kBodyTextLargePlatformHeight + kBodyHeight;
static const int default_height =
GetFont(CONTEXT_BODY_TEXT_SMALL, kTemplateStyle).GetHeight() -
kBodyTextSmallPlatformHeight + kBodyHeight;
switch (text_context) {
case CONTEXT_HEADLINE:
return headline_height;
case views::style::CONTEXT_DIALOG_TITLE:
return title_height;
case CONTEXT_BODY_TEXT_LARGE:
return body_large_height;
case views::style::CONTEXT_BUTTON:
return kButtonAbsoluteHeight;
default:
return default_height;
}
}
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROME_BROWSER_UI_VIEWS_HARMONY_HARMONY_TYPOGRAPHY_PROVIDER_H_
#define CHROME_BROWSER_UI_VIEWS_HARMONY_HARMONY_TYPOGRAPHY_PROVIDER_H_
#include "base/macros.h"
#include "ui/views/style/typography_provider.h"
// TypographyProvider implementing the Harmony spec.
class HarmonyTypographyProvider : public views::TypographyProvider {
public:
HarmonyTypographyProvider() = default;
// TypographyProvider:
const gfx::FontList& GetFont(int text_context, int text_style) const override;
SkColor GetColor(int context, int style) const override;
int GetLineHeight(int context, int style) const override;
private:
DISALLOW_COPY_AND_ASSIGN(HarmonyTypographyProvider);
};
#endif // CHROME_BROWSER_UI_VIEWS_HARMONY_HARMONY_TYPOGRAPHY_PROVIDER_H_
......@@ -7,6 +7,7 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/default_style.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/base/test/material_design_controller_test_api.h"
#include "ui/gfx/font_list.h"
#include "ui/views/style/typography.h"
#include "ui/views/style/typography_provider.h"
......@@ -229,3 +230,42 @@ TEST(LayoutDelegateTest, FontSizeRelativeToBase) {
GetFont(CONTEXT_DEPRECATED_SMALL, kStyle).GetFontSize());
#endif
}
// Ensure that line height can be overridden by Chrome's TypographyProvider for
// for the standard set of styles. This varies by platform and test machine
// configuration. Generally, for a particular platform configuration, there
// should be a consistent increase in line height when compared to the height of
// a given font.
TEST(LayoutDelegateTest, TypographyLineHeight) {
constexpr int kStyle = views::style::STYLE_PRIMARY;
// Only MD overrides the default line spacing.
ui::test::MaterialDesignControllerTestAPI md_test_api(
ui::MaterialDesignController::MATERIAL_NORMAL);
md_test_api.SetSecondaryUiMaterial(true);
ChromeViewsDelegate views_delegate;
constexpr struct {
int context;
int min;
int max;
} kExpectedIncreases[] = {{CONTEXT_HEADLINE, 4, 8},
{views::style::CONTEXT_DIALOG_TITLE, 2, 4},
{CONTEXT_BODY_TEXT_LARGE, 3, 4},
{CONTEXT_BODY_TEXT_SMALL, 5, 5}};
for (size_t i = 0; i < arraysize(kExpectedIncreases); ++i) {
SCOPED_TRACE(testing::Message() << "Testing index: " << i);
const auto& increase = kExpectedIncreases[i];
const gfx::FontList& font = views::style::GetFont(increase.context, kStyle);
int line_spacing = views::style::GetLineHeight(increase.context, kStyle);
EXPECT_GE(increase.max, line_spacing - font.GetHeight());
EXPECT_LE(increase.min, line_spacing - font.GetHeight());
}
// Buttons should specify zero line height (i.e. use the font's height) so
// buttons have flexibility to configure their own spacing.
EXPECT_EQ(0,
views::style::GetLineHeight(views::style::CONTEXT_BUTTON, kStyle));
}
......@@ -21,6 +21,16 @@ struct FontRenderParams;
class GFX_EXPORT PlatformFont : public base::RefCounted<PlatformFont> {
public:
// The size of the font returned by CreateDefault() on a "default" platform
// configuration. This allows UI that wants to target a particular size of font
// to obtain that size for the majority of users, while still compensating for a
// user preference for a larger or smaller font.
#if defined(OS_MACOSX)
static constexpr int kDefaultBaseFontSize = 13;
#else
static constexpr int kDefaultBaseFontSize = 12;
#endif
// Creates an appropriate PlatformFont implementation.
static PlatformFont* CreateDefault();
#if defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_IOS)
......
......@@ -234,6 +234,7 @@ gfx::Size LabelButton::GetPreferredSize() const {
// Use a temporary label copy for sizing to avoid calculation side-effects.
Label label(GetText(), {label_->font_list()});
label.SetLineHeight(label_->line_height());
label.SetShadows(label_->shadows());
if (style_ == STYLE_BUTTON && PlatformStyle::kDefaultLabelButtonHasBoldFont) {
......
......@@ -49,6 +49,7 @@ Label::Label(const base::string16& text)
Label::Label(const base::string16& text, int text_context, int text_style)
: context_menu_contents_(this) {
Init(text, style::GetFont(text_context, text_style));
SetLineHeight(style::GetLineHeight(text_context, text_style));
}
Label::Label(const base::string16& text, const CustomFont& font)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment