New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 645174 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Harmony - update Harmony button API to correspond to Harmony spec

Project Member Reported by shrike@chromium.org, Sep 8 2016

Issue description

The Harmony spec specifies two button types: Primary and Secondary. Primary buttons fill with blue (vs. white), and a secondary ui dialog can contain only one Primary button.

Before the Harmony spec there was a push to convert Chrome secondary UI to Material Design, which was the genesis of MdTextButton. That Material Design effort turned into Harmony, and so MdTextButton became the implementation behind Harmony buttons.

MdTextButton's blue button variant is called a "call to action" button. "Call to action" is an advertising term, not a UI term, so it should not have become part of the MdTextButton API (I think the Material Design changes were undertaken without a design doc). More importantly, if an engineer today is asked to implement a Harmony dialog that includes a Primary button, they must figure out that calling SetCallToAction(true) creates a Primary button. MdTextButton's API needs to be changed to match the Harmony spec, so that creating a Primary button is done with a straightforward and obvious call to SetPrimary(true).

Link to the Harmony sticker sheet showing the Primary and Secondary button styles:

https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)#%2F_P%20-%20Chrome%20-%20stickersheet.png%3Fz=width
 
Cc: bettes@chromium.org
Jayson says the code review issue is not the right place to debate this. There was already a debate on  bug 642797  which is oddly not referenced here. In any case, "call to action" is a visual style which is applied to the default button in dialogs. It is also used in the app list, where it's not unique, and in the chromeos system menu for the cast row, where it's not the "primary" action, just a useful button that we want to call attention to. You can think of a dialog's default action as a "primary" button if you like, but those "primary" buttons are just one place that uses call to action styling.

I think Alan should weigh in here.
As I have stated many times, "Call to action" is not a UI term, and so it should not be part of the API for a user interface component. But more importantly (to state again), the "call to action" API does not correspond to the Primary naming in the spec, and therefore it is confusing and needs to be changed.
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 28 2016

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

commit 49464ba3f03b6215d2c4f642d4bdf98a89b2d7df
Author: shrike <shrike@chromium.org>
Date: Tue Sep 27 23:08:35 2016

Add SetProminent() to MdTextButton to create blue buttons.

As agreed upon in https://groups.google.com/a/google.com/forum/#!topic/chrome-harmony/-VDL-TOtZVA
the API for configuring an MdTextButton to have a blue fill color shall
be SetProminent(). This cl updates the MdTextButton API to conform to this
decision.

R=sky@chromium.org
BUG= 645174 

Review-Url: https://codereview.chromium.org/2375543003
Cr-Commit-Position: refs/heads/master@{#421379}

[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/ash/common/system/chromeos/session/logout_button_tray.cc
[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/chrome/browser/ui/libgtk2ui/gtk2_ui.h
[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/chrome/browser/ui/libgtk2ui/native_theme_gtk2.cc
[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/chrome/browser/ui/views/infobars/confirm_infobar.cc
[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/chrome/browser/ui/views/location_bar/bubble_icon_view.cc
[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/ui/app_list/views/search_result_actions_view.cc
[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/ui/native_theme/common_theme.cc
[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/ui/native_theme/native_theme.h
[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/ui/native_theme/native_theme_dark_aura.cc
[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/ui/native_theme/native_theme_mac.mm
[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/ui/native_theme/native_theme_win.cc
[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/ui/views/controls/button/md_text_button.cc
[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/ui/views/controls/button/md_text_button.h
[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/ui/views/controls/button/toggle_button.cc
[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/ui/views/controls/progress_bar.cc
[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/ui/views/controls/tabbed_pane/tabbed_pane.cc
[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/ui/views/examples/button_example.cc
[modify] https://crrev.com/49464ba3f03b6215d2c4f642d4bdf98a89b2d7df/ui/views/examples/button_sticker_sheet.cc

Comment 4 by shrike@chromium.org, Sep 28 2016

Status: Fixed (was: Assigned)

Sign in to add a comment