"Bootstrap" a toolkit-views Typography spec.

ui/views/style/typography.h

Index: ui/views/style/typography.h
diff --git a/ui/views/style/typography.h b/ui/views/style/typography.h
new file mode 100644
index 0000000000000000000000000000000000000000..71b3b9cfab5bba22188b8ca972d934436247e8a5
--- /dev/null
+++ b/ui/views/style/typography.h
@@ -0,0 +1,138 @@
+// 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 UI_VIEWS_STYLE_TYPOGRAPHY_H_
+#define UI_VIEWS_STYLE_TYPOGRAPHY_H_
+
+#include <memory>
+
+#include "base/macros.h"
+#include "third_party/skia/include/core/SkColor.h"
+#include "ui/views/views_export.h"
+
+namespace gfx {
+class FontList;
+}
+
+namespace views {
+
+template <class ENUM>
+class ExtendableEnum {
+ public:
+  class Value;
+  explicit constexpr ExtendableEnum(int value) : value_(value) {}
+  constexpr int value() const { return value_; }
+
+ private:
+  int value_;
+};
+
+template <class ENUM>
+class ExtendableEnum<ENUM>::Value : public ExtendableEnum<ENUM> {
+ public:
+  // Allows use in switch statements.
+  constexpr operator int() const { return this->value(); }
+
+  // Allow implicit initialization from an integer literal.
+  constexpr Value(int const_value) : ExtendableEnum<ENUM>(const_value) {}
+};
+
+// Where a piece of text appears in the UI. This influences size and weight, but
+// typically not style or color.
+class VIEWS_EXPORT TextContext : public ExtendableEnum<TextContext> {

[tapted, 2017/03/15 00:30:05]

Still working on the rest, but I came up with this `ExtendableEnum` to solve the
layering while keeping the type safety and enum syntax. I think it will work...
(clang likes it :).

As far as "client" code goes, I think the only thing lost is the compiler
warning "value Foo not handled in switch", but that's unavoidable. Otherwise,
the switch syntax works just like an enum.

For declaring the `ExtendableEnum` the syntax is a bit more verbose (almost
Java-esque) and needs explicit initializers, but that's not too bad.

I guess questions at this stage would be
 - Should I proceed with it for TextContext/TextStyle?
 - Is where I've "extended" it in chrome/../layout_delegate.h the right place?
(chrome_typography.h maybe?)
 - Should I move ExtendableEnum to /src/base?


(no worries if you don't see this - I'll figure out something sensible)

[Peter Kasting, 2017/03/15 06:38:48]

Hmm.  My take on this is that overall this feels more verbose to me than the
content/ method (of two enums), by such a large degree that it's hard to read
and not worth doing, but that's also affected by my suspicion that the
compile-time typechecking from this will never catch any real bugs (and
therefore the benefits are low).

It's clever though :)

[tapted, 2017/03/15 08:26:25]

The problem I see is that TextContext and TextStyle are (mostly)orthogonal
concepts, but it they are both typedef to `int` it will be really easy to get
them swapped around. On top of this, there are a lot of `default:` cases, and
subtle differences, so it will be easy to get something that even looks correct
when run, but may cause subtle and hard to diagnose bugs if we later decide to
tweak the typographyprovider (it may even depend on things from ui::NativeTheme,
so not even show up on test computers).

[Peter Kasting, 2017/03/15 10:01:48]

Hmm.  The confusion between those two is a fair thing to avoid.

When coupled with the issues with getting the correct provider (and my desire
not to provide a setter), this would push me to split the class into two halves
instead.  It just feels so gross :(.  Maybe I'm overreacting, I'm tired so my
judgment is questionable.

[tapted, 2017/03/15 11:04:07]

I think if we split the class like that we never be able to pass enum values
from the chrome "half" to a views::Label constructor and have it ask the
delegate on our behalf -- we'd go back to requiring manual and separate
configuration of font size, font weight, color, line height every time we create
a new views::Label.

(we could inherit from views::Label, or have ViewsDelegate provide a
"LabelFactory"..  but that's a whole world of other problems :/)

[tapted, 2017/03/15 22:43:35]

I dreamed up a way that would let us have an enum and let us `DCHECK` that they
haven't been swapped around. I think that will satisfy me enough to drop the
type safety (and I get the impression nobody's really excited about
ExtendableEnum).

We can merely require that all TextStyle enum values are greater than, say,
10,000, and require TypographyProviders to check.

[Peter Kasting, 2017/03/16 00:17:47]

Yeah, that'd work.

Heck, make it a bitfield and just OR them all together into one number.  :D

+ public:
+  // Headline text. Usually 20pt. Never multi-line.
+  static constexpr const Value HEADLINE = 1;
+
+  // A title for a dialog window. Usually 15pt. Multi-line OK.
+  static constexpr const Value DIALOG_TITLE = 2;
+
+  // "Body 1". Usually 13pt.
+  static constexpr const Value DIALOG_MESSAGE = 3;
+
+  // "Body 2". Usually 12pt.
+  static constexpr const Value CONTROL_LABEL = 4;
+
+  // A text field. Usually matches CONTROL_LABEL.
+  static constexpr const Value FIELD = 5;
+
+  // Text that appears on a button control. Usually 12pt.
+  static constexpr const Value BUTTON_TEXT = 6;
+
+  // Allow implicit conversion from the constants above.
+  constexpr TextContext(const Value& value) : ExtendableEnum(value.value()) {}
+
+  // Allow declaration without assignment.
+  constexpr TextContext() : ExtendableEnum(0) {}
+
+ protected:
+  enum { VIEWS_MAX = 20 };
+};
+
+// How a piece of text should be presented. This influences color and style, but
+// typically not size.
+enum class TextStyle {
+  // Primary text: solid black, normal weight. Converts to DISABLED in some
+  // contexts (e.g. BUTTON_TEXT, FIELD).
+  PRIMARY,
+
+  // Secondary text: may be lighter.
+  SECONDARY,
+
+  // "Hint" text, usually a line that gives context to something more important.
+  HINT,
+
+  DISABLED,  // Disabled "greyed out" text.
+  RED,       // A solid shade of red.
+  GREEN,     // A solid shade of green.
+  LINK,      // A solid shade of blue.
+
+  // Legacy styles start here.
+
+  ACTIVE_TAB,      // Active tab in a tabbed pane.
+  INACTIVE_TAB,    // Inactive tab in a tabbed pane.
+  HOVERED_TAB,     // Hovered tab in a tabbed pane.
+  WEB_DOMAIN,      // Used to draw attention to a web domain.
+  EXTENSION_NAME,  // Used to draw attention to an extension name.
+};
+
+// Provides fonts to use in toolkit-views UI.
+class VIEWS_EXPORT TypographyProvider {
+ public:
+  // Gets the FontList for the given |context| and |style|.
+  virtual const gfx::FontList& GetFont(TextContext context,
+                                       TextStyle style) = 0;
+
+  // Gets the color for the given |context| and |style|. This may consult
+  // ui::NativeTheme.
+  virtual SkColor GetColor(TextContext context, TextStyle style) = 0;
+
+  // Gets the line spacing, or 0 if it should be provided by gfx::FontList.
+  virtual int GetLineSpacing(TextContext context, TextStyle style) = 0;
+
+  virtual ~TypographyProvider() {}
+
+ protected:
+  TypographyProvider() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TypographyProvider);
+};
+
+// Helper class to query (or change) the current TypographyProvider.
+class VIEWS_EXPORT Typography {
+ public:
+  static void SetProvider(std::unique_ptr<TypographyProvider> provider);
+
+  static const gfx::FontList& GetFont(TextContext context, TextStyle style);
+  static SkColor GetColor(TextContext context, TextStyle style);
+  static int GetLineSpacing(TextContext context, TextStyle style);
+
+ private:
+  DISALLOW_IMPLICIT_CONSTRUCTORS(Typography);
+};
+
+}  // namespace views
+
+#endif  // UI_VIEWS_STYLE_TYPOGRAPHY_PROVIDER_H_