Chromium Code Reviews
Help | Chromium Project | Sign in
(9)

Issue 2684343006: Make the account chooser and CVC dialog use the same icon with toolip for Views. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 2 weeks ago by vasilii
Modified:
2 months, 1 week ago
Reviewers:
msw, Evan Stade
CC:
chromium-reviews, vabr+watchlistpasswordmanager_chromium.org, rouslan+autofill_chromium.org, rogerm+autofillwatch_chromium.org, sebsg+autofillwatch_chromium.org, mathp+autofillwatch_chromium.org, tfarina, vabr+watchlistautofill_chromium.org, estade+watch_chromium.org, gcasto+watchlist_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Make the account chooser and CVC dialog use the same icon with toolip for Views. The CL changes the icon to (i). The account chooser reuses the implementation of the icon and the tooltip for the CVC dialog. BUG=690920 Review-Url: https://codereview.chromium.org/2684343006 Cr-Commit-Position: refs/heads/master@{#451273} Committed: https://chromium.googlesource.com/chromium/src/+/864f2986aa206bae957944685e1bbb0d7b4fdcc4

Patch Set 1 #

Total comments: 4

Patch Set 2 : move files #

Total comments: 56

Patch Set 3 : comments from msw@ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+158 lines, -636 lines) Patch
M chrome/browser/ui/BUILD.gn View 1 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/ui/views/autofill/card_unmask_prompt_views.cc View 1 2 3 chunks +7 lines, -3 lines 0 comments Download
D chrome/browser/ui/views/autofill/info_bubble.h View 1 1 chunk +0 lines, -77 lines 0 comments Download
D chrome/browser/ui/views/autofill/info_bubble.cc View 1 1 chunk +0 lines, -154 lines 0 comments Download
D chrome/browser/ui/views/autofill/tooltip_icon.h View 1 1 chunk +0 lines, -87 lines 0 comments Download
M chrome/browser/ui/views/autofill/tooltip_icon.cc View 1 1 chunk +0 lines, -148 lines 0 comments Download
M chrome/browser/ui/views/passwords/credentials_item_view.h View 1 2 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/ui/views/passwords/credentials_item_view.cc View 1 2 6 chunks +5 lines, -20 lines 0 comments Download
M ui/views/BUILD.gn View 1 1 chunk +4 lines, -0 lines 0 comments Download
A ui/views/bubble/info_bubble.h View 1 2 1 chunk +60 lines, -0 lines 0 comments Download
A + ui/views/bubble/info_bubble.cc View 1 2 5 chunks +37 lines, -62 lines 0 comments Download
A + ui/views/bubble/tooltip_icon.h View 1 2 2 chunks +20 lines, -23 lines 0 comments Download
A + ui/views/bubble/tooltip_icon.cc View 1 2 7 chunks +22 lines, -55 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 33 (17 generated)
vasilii
Hi Evan, I have few questions - What's the magic constant '233' in info_bubble.cc? Due ...
2 months, 2 weeks ago (2017-02-10 14:15:16 UTC) #4
Evan Stade
On 2017/02/10 14:15:16, vasilii wrote: > Hi Evan, > > I have few questions > ...
2 months, 2 weeks ago (2017-02-10 15:43:38 UTC) #7
vasilii
> > - The Enamel team wants the icon to consume the click. It's not ...
2 months, 2 weeks ago (2017-02-10 16:54:22 UTC) #8
vasilii
> Not user visible? Wouldn't it mean that clicking no longer focuses the cvc > ...
2 months, 2 weeks ago (2017-02-10 18:33:20 UTC) #9
Evan Stade
I may be misremembering but I thought the icon was inside the text field. I'm ...
2 months, 2 weeks ago (2017-02-10 19:06:15 UTC) #10
vasilii
On 2017/02/10 19:06:15, Evan Stade wrote: > I may be misremembering but I thought the ...
2 months, 1 week ago (2017-02-13 15:26:51 UTC) #11
vasilii
https://codereview.chromium.org/2684343006/diff/1/chrome/browser/ui/views/passwords/credentials_item_view.cc File chrome/browser/ui/views/passwords/credentials_item_view.cc (right): https://codereview.chromium.org/2684343006/diff/1/chrome/browser/ui/views/passwords/credentials_item_view.cc#newcode66 chrome/browser/ui/views/passwords/credentials_item_view.cc:66: }; On 2017/02/10 15:43:38, Evan Stade wrote: > disallow ...
2 months, 1 week ago (2017-02-15 16:29:21 UTC) #14
vasilii
Hi Mike, please review the files added to ui/views/bubble. I think it's the right place.
2 months, 1 week ago (2017-02-15 16:34:04 UTC) #16
msw
Have we considered just using standard views tooltips? https://codereview.chromium.org/2684343006/diff/20001/chrome/browser/ui/views/autofill/card_unmask_prompt_views.cc File chrome/browser/ui/views/autofill/card_unmask_prompt_views.cc (right): https://codereview.chromium.org/2684343006/diff/20001/chrome/browser/ui/views/autofill/card_unmask_prompt_views.cc#newcode239 chrome/browser/ui/views/autofill/card_unmask_prompt_views.cc:239: icon->set_bubble_width(233); ...
2 months, 1 week ago (2017-02-15 21:06:39 UTC) #19
vasilii
The standard tooltip is used now in the account chooser. There was a request from ...
2 months, 1 week ago (2017-02-16 14:28:07 UTC) #20
msw
I would *highly* encourage use of the standard Views tooltip, as one-off solutions like this ...
2 months, 1 week ago (2017-02-16 19:04:29 UTC) #25
Evan Stade
On 2017/02/16 19:04:29, msw wrote: > I would *highly* encourage use of the standard Views ...
2 months, 1 week ago (2017-02-16 19:16:23 UTC) #26
hwi1
On 2017/02/16 19:04:29, msw wrote: > I would *highly* encourage use of the standard Views ...
2 months, 1 week ago (2017-02-17 04:58:49 UTC) #27
hwi1
On 2017/02/16 19:16:23, Evan Stade wrote: > On 2017/02/16 19:04:29, msw wrote: > > I ...
2 months, 1 week ago (2017-02-17 05:09:07 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2684343006/40001
2 months, 1 week ago (2017-02-17 09:41:02 UTC) #30
commit-bot: I haz the power
2 months, 1 week ago (2017-02-17 09:45:51 UTC) #33
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/864f2986aa206bae957944685e1b...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46