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

Issue 10270016: Fix spacing around left tab, increment theme resource ID (Closed)

Created:
8 years, 7 months ago by James Cook
Modified:
8 years, 7 months ago
CC:
chromium-reviews
Visibility:
Public.

Description

Fix spacing around left tab, increment theme resource ID * Adjust spacing between incognito man and leftmost tab. * Make pinned tabs closer to their width in R19. * Increment theme pack ID so tab background images get rebuilt. BUG=125005 TEST=visual Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=134560

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -15 lines) Patch
M chrome/browser/defaults.cc View 2 chunks +7 lines, -2 lines 0 comments Download
M chrome/browser/themes/browser_theme_pack.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/frame/glass_browser_frame_view.cc View 4 chunks +7 lines, -6 lines 0 comments Download
M chrome/browser/ui/views/frame/opaque_browser_frame_view.cc View 4 chunks +7 lines, -6 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
James Cook
Ben, some metrics tweaks to clean up the spacing for the Ash tab strip assets ...
8 years, 7 months ago (2012-04-30 16:45:44 UTC) #1
Ben Goodger (Google)
8 years, 7 months ago (2012-04-30 17:01:17 UTC) #2
LGTM

On Mon, Apr 30, 2012 at 9:45 AM, <jamescook@chromium.org> wrote:

> Reviewers: Ben Goodger (Google),
>
> Message:
> Ben, some metrics tweaks to clean up the spacing for the Ash tab strip
> assets on
> Windows.
>
>
> Description:
> Fix spacing around left tab, increment theme resource ID
>
> * Adjust spacing between incognito man and leftmost tab.
> * Make pinned tabs closer to their width in R19.
> * Increment theme pack ID so tab background images get rebuilt.
>
> BUG=125005
> TEST=visual
>
>
> Please review this at
http://codereview.chromium.**org/10270016/<http://codereview.chromium.org/102...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>  M chrome/browser/defaults.cc
>  M chrome/browser/themes/browser_**theme_pack.cc
>  M chrome/browser/ui/views/frame/**glass_browser_frame_view.cc
>  M chrome/browser/ui/views/frame/**opaque_browser_frame_view.cc
>
>
> Index: chrome/browser/defaults.cc
> diff --git a/chrome/browser/defaults.cc b/chrome/browser/defaults.cc
> index 6568e52725f88a5c8d6df6f2893c6d**1c3da630c2..**
> 0e4e057daa416b784c36b12f0d7c02**9119b303a8 100644
> --- a/chrome/browser/defaults.cc
> +++ b/chrome/browser/defaults.cc
> @@ -25,7 +25,6 @@ const int kAutocompleteEditFontPixelSize = 15;
>
>  const int kAutocompleteEditFontPixelSize**InPopup = 10;
>
> -const int kMiniTabWidth = 64;
>  const bool kCanToggleSystemTitleBar = false;
>  const bool kRestorePopups = false;
>  const bool kShowImportOnBookmarkBar = false;
> @@ -58,9 +57,15 @@ const bool kCanToggleSystemTitleBar = true;
>
>  #endif  // defined(OS_CHROMEOS)
>
> +#if defined(TOOLKIT_VIEWS)
> +// Windows and Chrome OS have bigger shadows in the tab art.
> +const int kMiniTabWidth = 64;
> +#else
> +const int kMiniTabWidth = 56;
> +#endif  // defined(TOOLKIT_VIEWS)
> +
>  #if !defined(OS_CHROMEOS)
>
> -const int kMiniTabWidth = 56;
>  const bool kRestorePopups = false;
>  const bool kShowImportOnBookmarkBar = true;
>  const bool kDownloadPageHasShowInFolder = true;
> Index: chrome/browser/themes/browser_**theme_pack.cc
> diff --git a/chrome/browser/themes/**browser_theme_pack.cc
> b/chrome/browser/themes/**browser_theme_pack.cc
> index 980a1a411179bf9b10ea58a6f147b5**91a2c1fdd8..**
> 5d8fd739ee3412db0f3829baeaca38**7309a93956 100644
> --- a/chrome/browser/themes/**browser_theme_pack.cc
> +++ b/chrome/browser/themes/**browser_theme_pack.cc
> @@ -35,7 +35,7 @@ namespace {
>  // Version number of the current theme pack. We just throw out and rebuild
>  // theme packs that aren't int-equal to this. Increment this number if you
>  // change default theme assets.
> -const int kThemePackVersion = 20;
> +const int kThemePackVersion = 21;
>
>  // IDs that are in the DataPack won't clash with the positive integer
>  // uint16. kHeaderID should always have the maximum value because we want
> the
> Index: chrome/browser/ui/views/frame/**glass_browser_frame_view.cc
> diff --git a/chrome/browser/ui/views/**frame/glass_browser_frame_**view.cc
> b/chrome/browser/ui/views/**frame/glass_browser_frame_**view.cc
> index 180a6b6ff06ce6af0fc6dcf1fa8690**a9f9453de6..**
> 4121596cb967f8256c1b0ed107130a**ce09fb2c61 100644
> --- a/chrome/browser/ui/views/**frame/glass_browser_frame_**view.cc
> +++ b/chrome/browser/ui/views/**frame/glass_browser_frame_**view.cc
> @@ -46,9 +46,10 @@ const int kResizeAreaCornerSize = 16;
>  // way the tabstrip draws its bottom edge, will appear like a 1 px gap to
> the
>  // user).
>  const int kAvatarBottomSpacing = 2;
> -// There are 2 px on each side of the avatar (between the frame border and
> -// it on the left, and between it and the tabstrip on the right).
> -const int kAvatarSideSpacing = 2;
> +// Space between the frame border and the left edge of the avatar.
> +const int kAvatarLeftSpacing = 2;
> +// Space between the right edge of the avatar and the tabstrip.
> +const int kAvatarRightSpacing = -2;
>  // The content left/right images have a shadow built into them.
>  const int kContentEdgeShadowThickness = 2;
>  // The top 3 px of the tabstrip is shadow; in maximized mode we push this
> off
> @@ -64,7 +65,7 @@ const int kNewTabCaptionRestoredSpacing = 5;
>  const int kNewTabCaptionMaximizedSpacing = 16;
>  // How far to indent the tabstrip from the left side of the screen when
> there
>  // is no avatar icon.
> -const int kTabStripIndent = -4;
> +const int kTabStripIndent = -6;
>  }
>
>  //////////////////////////////**//////////////////////////////**
> ///////////////////
> @@ -96,7 +97,7 @@ gfx::Rect GlassBrowserFrameView::**GetBoundsForTabStrip(
>   int minimize_button_offset =
>       std::min(frame()->**GetMinimizeButtonOffset(), width());
>   int tabstrip_x = browser_view()->**ShouldShowAvatar() ?
> -      (avatar_bounds_.right() + kAvatarSideSpacing) :
> +      (avatar_bounds_.right() + kAvatarRightSpacing) :
>       NonClientBorderThickness() + kTabStripIndent;
>   // In RTL languages, we have moved an avatar icon left by the size of
> window
>   // controls to prevent it from being rendered over them. So, we use its x
> @@ -386,7 +387,7 @@ void GlassBrowserFrameView::**LayoutAvatar() {
>   // can be customized so we can't depend on its size to perform layout.
>   SkBitmap incognito_icon = browser_view()->**GetOTRAvatarIcon();
>
> -  int avatar_x = NonClientBorderThickness() + kAvatarSideSpacing;
> +  int avatar_x = NonClientBorderThickness() + kAvatarLeftSpacing;
>   // Move this avatar icon by the size of window controls to prevent it
> from
>   // being rendered over them in RTL languages. This code also needs to
> adjust
>   // the width of a tab strip to avoid decreasing this size twice. (See the
> Index: chrome/browser/ui/views/frame/**opaque_browser_frame_view.cc
> diff --git a/chrome/browser/ui/views/**frame/opaque_browser_frame_**view.cc
> b/chrome/browser/ui/views/**frame/opaque_browser_frame_**view.cc
> index b98c7f3e9b31076827581068ab9953**4ccc0fd5bf..**
> f4d82c87bc0da599c9af762a815185**cc90cff063 100644
> --- a/chrome/browser/ui/views/**frame/opaque_browser_frame_**view.cc
> +++ b/chrome/browser/ui/views/**frame/opaque_browser_frame_**view.cc
> @@ -84,9 +84,10 @@ const int kTitleLogoSpacing = 5;
>  // way the tabstrip draws its bottom edge, will appear like a 1 px gap to
> the
>  // user).
>  const int kAvatarBottomSpacing = 2;
> -// There are 2 px on each side of the avatar (between the frame border and
> -// it on the left, and between it and the tabstrip on the right).
> -const int kAvatarSideSpacing = 2;
> +// Space between the frame border and the left edge of the avatar.
> +const int kAvatarLeftSpacing = 2;
> +// Space between the right edge of the avatar and the tabstrip.
> +const int kAvatarRightSpacing = -2;
>  // The top 3 px of the tabstrip is shadow; in maximized mode we push this
> off
>  // the top of the screen so the tabs appear flush against the screen edge.
>  const int kTabstripTopShadowThickness = 3;
> @@ -100,7 +101,7 @@ const int kNewTabCaptionRestoredSpacing = 5;
>  const int kNewTabCaptionMaximizedSpacing = 16;
>  // How far to indent the tabstrip from the left side of the screen when
> there
>  // is no avatar icon.
> -const int kTabStripIndent = -4;
> +const int kTabStripIndent = -6;
>
>  // Converts |bounds| from |src|'s coordinate system to |dst|, and checks
> if
>  // |pt| is contained within.
> @@ -210,7 +211,7 @@ gfx::Rect OpaqueBrowserFrameView::**
> GetBoundsForTabStrip(
>     return gfx::Rect();
>
>   int tabstrip_x = browser_view()->**ShouldShowAvatar() ?
> -      (avatar_bounds_.right() + kAvatarSideSpacing) :
> +      (avatar_bounds_.right() + kAvatarRightSpacing) :
>       NonClientBorderThickness() + kTabStripIndent;
>
>   int maximized_spacing = kNewTabCaptionMaximizedSpacing**;
> @@ -957,7 +958,7 @@ void OpaqueBrowserFrameView::**LayoutAvatar() {
>   int avatar_y = frame()->IsMaximized() ?
>       (NonClientTopBorderHeight(**false) + kTabstripTopShadowThickness) :
>       avatar_restored_y;
> -  avatar_bounds_.SetRect(**NonClientBorderThickness() +
> kAvatarSideSpacing,
> +  avatar_bounds_.SetRect(**NonClientBorderThickness() +
> kAvatarLeftSpacing,
>       avatar_y, incognito_icon.width(),
>       browser_view()->**ShouldShowAvatar() ? (avatar_bottom - avatar_y) :
> 0);
>
>
>
>

Powered by Google App Engine
This is Rietveld 408576698