Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(312)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 11818044: Replace IDR_WEB_UI_CLOSE* with IDR_CLOSE_DIALOG*, cleanup, etc. (Closed)

Created:
7 years, 11 months ago by msw
Modified:
7 years, 11 months ago
Reviewers:
sky
CC:
chromium-reviews, groby+watch_chromium.org, nkostylev+watch_chromium.org, tfarina, rouslan+watch_chromium.org, sail+watch_chromium.org, arv (Not doing code reviews), oshima+watch_chromium.org, gbillock+watch_chromium.org, smckay+watch_chromium.org, stevenjb+watch_chromium.org
Visibility:
Public.

Description

Replace IDR_WEB_UI_CLOSE* with IDR_CLOSE_DIALOG*, cleanup, etc. Replace IDR_WEB_UI_CLOSE* uses with IDR_CLOSE_DIALOG*. (same images from ui/resources instead of chrome/app/theme) Remove IDR_WEB_UI_CLOSE* IDRs and web_ui_close*.png images. Give ConfirmBubbleViews's close button hover/pressed images. (this better matches the specification for the new dialog style) Note: To test the only use of ConfirmBubbleViews: 1) Open a webpage textfield context menu (right-click). 2) Select "Spell-checker options" -> "Ask Google for suggestions". 3) Observe the dialog's close button on hover/press. Replace the LabelButtonBorder in LabelButton::SetNativeTheme. (instead of casting a potentially non-existant/different border) Cap the label width to a non-negative size in LabelButton::Layout. (to prevent crashes when the image size exceeds the available size) Remove the DialogFrameView close button border, adjust Layout. (this better matches the specification for the new dialog style) See the resulting screenshot at: http://crbug.com/166075#c11 BUG=166075 TEST=Spelling bubble has better (x) image; no build breaks. R=sky@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=176265

Patch Set 1 #

Total comments: 4

Patch Set 2 : Address comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+44 lines, -37 lines) Patch
D chrome/app/theme/default_100_percent/web_ui_close.png View 0 chunks +-1 lines, --1 lines 0 comments Download
D chrome/app/theme/default_100_percent/web_ui_close_hover.png View 0 chunks +-1 lines, --1 lines 0 comments Download
D chrome/app/theme/default_100_percent/web_ui_close_pressed.png View 0 chunks +-1 lines, --1 lines 0 comments Download
D chrome/app/theme/default_200_percent/web_ui_close.png View 0 chunks +-1 lines, --1 lines 0 comments Download
D chrome/app/theme/default_200_percent/web_ui_close_hover.png View 0 chunks +-1 lines, --1 lines 0 comments Download
D chrome/app/theme/default_200_percent/web_ui_close_pressed.png View 0 chunks +-1 lines, --1 lines 0 comments Download
M chrome/app/theme/theme_resources.grd View 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/browser/resources/chromeos/login/oobe.css View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/resources/print_preview/no_destinations_promo.css View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/resources/print_preview/search/destination_search.css View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/resources/shared/css/dialogs.css View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/resources/shared/css/overlay.css View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/ui/chrome_style.cc View 1 1 chunk +5 lines, -6 lines 0 comments Download
M chrome/browser/ui/cocoa/hover_close_button.mm View 2 chunks +5 lines, -4 lines 0 comments Download
M chrome/browser/ui/views/confirm_bubble_views.cc View 1 3 chunks +9 lines, -6 lines 0 comments Download
M chrome/browser/ui/views/web_intent_picker_views.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M ui/views/controls/button/label_button.cc View 2 chunks +5 lines, -2 lines 0 comments Download
M ui/views/window/dialog_frame_view.cc View 2 chunks +8 lines, -4 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
msw
Hey Scott, please take a look; thanks!
7 years, 11 months ago (2013-01-10 19:30:43 UTC) #1
tfarina
https://codereview.chromium.org/11818044/diff/1/chrome/browser/ui/chrome_style.cc File chrome/browser/ui/chrome_style.cc (right): https://codereview.chromium.org/11818044/diff/1/chrome/browser/ui/chrome_style.cc#newcode15 chrome/browser/ui/chrome_style.cc:15: ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance(); supernit: while here, you could ...
7 years, 11 months ago (2013-01-10 19:32:48 UTC) #2
tfarina
Also, just a suggestion, may be IDR_CLOSE_BUTTON is cleaner?
7 years, 11 months ago (2013-01-10 19:34:11 UTC) #3
msw
Done. https://codereview.chromium.org/11818044/diff/1/chrome/browser/ui/chrome_style.cc File chrome/browser/ui/chrome_style.cc (right): https://codereview.chromium.org/11818044/diff/1/chrome/browser/ui/chrome_style.cc#newcode15 chrome/browser/ui/chrome_style.cc:15: ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance(); On 2013/01/10 19:32:48, tfarina ...
7 years, 11 months ago (2013-01-10 19:59:49 UTC) #4
sky
LGTM
7 years, 11 months ago (2013-01-10 21:33:41 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msw@chromium.org/11818044/8001
7 years, 11 months ago (2013-01-10 22:33:46 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msw@chromium.org/11818044/8001
7 years, 11 months ago (2013-01-11 03:38:31 UTC) #7
commit-bot: I haz the power
7 years, 11 months ago (2013-01-11 05:09:08 UTC) #8
Message was sent while issue was closed. 
Change committed as 176265

Powered by Google App Engine
This is Rietveld 408576698