New issue
Advanced search Search tips

Issue 814477 link

Starred by 1 user
Status: Fixed
Owner:
anthonyvd@chromium.org
Closed: Mar 2018
Cc:
yyushkina@chromium.org
ellyjo...@chromium.org
mac-bugs-priority@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug

Blocking:
issue 730958



Sign in to add a comment

New Translate UX on Mac fixes

Project Member Reported by anthonyvd@chromium.org, Feb 21 2018 
After talking with ellyjones@ and yyushkina@, it looks like the following issues should be fixed before launching.

1- Button order is wrong: the blue button goes on the right on Mac.

2- The menu is a Views menu instead of an NSMenu.

3- The "Options" button did not look like it should create a menu - on Mac at least, it should have a "disclosure arrow" (a downward-pointing arrow like a combobox does) or similar to indicate that it does that.

4- We need to fix the logic in TranslateService::IsTranslateBubbleEnabled to always use the Views Translate UI when MacViews secondary UI is enabled.
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 28 2018 

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

commit 9ee5170bc201ac7f07c44bd033359f62ad65abdc
Author: Anthony Vallee-Dubois <anthonyvd@chromium.org>
Date: Wed Feb 28 18:07:15 2018

Fix Mac button order in Translate bubble.

This CL exposes DialogClientView's kIsOkButtonOnLeftSide constant in
views::PlatformStyle and uses it in the views:: implementation of
TranslateBubble to provide the proper button order in the bubble.

Bug:  814477 
Change-Id: I2e4c9576fba01bd270310db7d483eb6a6327b192
Reviewed-on: https://chromium-review.googlesource.com/937884
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: anthonyvd <anthonyvd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539881}
[modify] https://crrev.com/9ee5170bc201ac7f07c44bd033359f62ad65abdc/chrome/browser/ui/views/translate/translate_bubble_view.cc
[modify] https://crrev.com/9ee5170bc201ac7f07c44bd033359f62ad65abdc/ui/views/style/platform_style.cc
[modify] https://crrev.com/9ee5170bc201ac7f07c44bd033359f62ad65abdc/ui/views/style/platform_style.h
[modify] https://crrev.com/9ee5170bc201ac7f07c44bd033359f62ad65abdc/ui/views/window/dialog_client_view.cc
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 1 2018 

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

commit 87d7d7e6b6e4c9da164f9efb5b083de6e023e482
Author: Anthony Vallee-Dubois <anthonyvd@chromium.org>
Date: Thu Mar 01 00:10:26 2018

Change Mac Translate Bubble options menu to a Cocoa menu

This CL also enables the MacViews translate bubble when
--secondary-ui-md is enabled.

Bug:  814477 
Change-Id: I107edf4fcbfde529b60433873c8e785a4fdbc2bf
Reviewed-on: https://chromium-review.googlesource.com/939521
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Rachel Blum <groby@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: anthonyvd <anthonyvd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539951}
[modify] https://crrev.com/87d7d7e6b6e4c9da164f9efb5b083de6e023e482/chrome/browser/about_flags.cc
[modify] https://crrev.com/87d7d7e6b6e4c9da164f9efb5b083de6e023e482/chrome/browser/autofill/autofill_interactive_uitest.cc
[modify] https://crrev.com/87d7d7e6b6e4c9da164f9efb5b083de6e023e482/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/87d7d7e6b6e4c9da164f9efb5b083de6e023e482/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/87d7d7e6b6e4c9da164f9efb5b083de6e023e482/chrome/browser/translate/translate_browsertest.cc
[modify] https://crrev.com/87d7d7e6b6e4c9da164f9efb5b083de6e023e482/chrome/browser/translate/translate_service.cc
[modify] https://crrev.com/87d7d7e6b6e4c9da164f9efb5b083de6e023e482/chrome/browser/ui/views/translate/translate_bubble_view.cc
[modify] https://crrev.com/87d7d7e6b6e4c9da164f9efb5b083de6e023e482/chrome/browser/ui/views/translate/translate_bubble_view_browsertest.cc
[modify] https://crrev.com/87d7d7e6b6e4c9da164f9efb5b083de6e023e482/chrome/common/chrome_switches.cc
[modify] https://crrev.com/87d7d7e6b6e4c9da164f9efb5b083de6e023e482/chrome/common/chrome_switches.h

Comment 3 by yyushkina@chromium.org, Mar 7 2018

Blocking: 730958

Comment 4 by yyushkina@chromium.org, Mar 7 2018

Labels: -Pri-2 Pri-1

Comment 5 by ellyjo...@chromium.org, Mar 23 2018

Labels: Target-66 MacViews-Dialogs
MacViews triage: anthonyvd: how is this looking?

Comment 6 by anthonyvd@chromium.org, Mar 27 2018

Status: Fixed (was: Assigned)
This is done % the disclosure arrow tracked in Issue 819768.
►

Sign in to add a comment