Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(11)

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:
4 months, 1 week ago by vasilii
Modified:
4 months ago
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 ...
4 months, 1 week 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 > ...
4 months, 1 week ago (2017-02-10 15:43:38 UTC) #7
vasilii
> > - The Enamel team wants the icon to consume the click. It's not ...
4 months, 1 week ago (2017-02-10 16:54:22 UTC) #8
vasilii
> Not user visible? Wouldn't it mean that clicking no longer focuses the cvc > ...
4 months, 1 week 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 ...
4 months, 1 week 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 ...
4 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 ...
4 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.
4 months, 1 week ago (2017-02-15 16:34:04 UTC) #16
msw - vacation june 8-27
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); ...
4 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 ...
4 months ago (2017-02-16 14:28:07 UTC) #20
msw - vacation june 8-27
I would *highly* encourage use of the standard Views tooltip, as one-off solutions like this ...
4 months 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 ...
4 months 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 ...
4 months 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 ...
4 months 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
4 months ago (2017-02-17 09:41:02 UTC) #30
commit-bot: I haz the power
4 months 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 cb946e318