New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 854479 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 28
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Feature

Blocked on:
issue 647989
issue 862045



Sign in to add a comment

Bring Windows PWA window frames up to feature parity with ChromeOS

Project Member Reported by alancutter@chromium.org, Jun 20 2018

Issue description

UI Mock for Windows 10:
https://docs.google.com/presentation/d/1hBKxbAt8Lv6fjHyp97PqLamCIXlSIF_sMmWyZnMob7M/edit#slide=id.g3bdd892e29_1_75

Currently Windows 10 PWA titlebars contain:
 - Icon
 - Page title
 - Chrome theme (if set)
 - Window controls

We need to:
 - Remove icon
 - Add conditional back button
 - Remove Chrome theming
 - Add app manifest theme colour background
 - Add origin text animation
 - Add app menu button
 - Add page action icons
 - Add extension action icons
 - Add content setting icons

 
Blockedon: 647989
Blocking: 851845
Description: Show this description
WIP screenshots.
before.png
8.9 KB View Download
hamb.png
8.1 KB View Download
Blockedon: 862045
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 13

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

commit 3f9933ea300cdd3bc77e927c2328f6ce55468bef
Author: Alan Cutter <alancutter@chromium.org>
Date: Fri Jul 13 05:24:41 2018

Add hosted app menu buttons to Windows 10 hosted app titlebars

This CL adds the hosted app menu button and content setting
icons to hosted app windows on Windows 10.
This change is hidden behind the DesktopPWAWindowing
feature flag.

Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=347457&signed_aid=au-e34qQfQGuYJ_kA_12hQ==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=347458&signed_aid=GB7N2rD2QIOLiOf0dnnBEg==&inline=1

Bug:  854479 
Change-Id: I719e96f1f1c75b8d2fa3e70a6597534101bde3a9
Reviewed-on: https://chromium-review.googlesource.com/1128824
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574844}
[modify] https://crrev.com/3f9933ea300cdd3bc77e927c2328f6ce55468bef/chrome/browser/ui/extensions/hosted_app_browser_controller.cc
[modify] https://crrev.com/3f9933ea300cdd3bc77e927c2328f6ce55468bef/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
[modify] https://crrev.com/3f9933ea300cdd3bc77e927c2328f6ce55468bef/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/3f9933ea300cdd3bc77e927c2328f6ce55468bef/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
[modify] https://crrev.com/3f9933ea300cdd3bc77e927c2328f6ce55468bef/chrome/browser/ui/views/frame/glass_browser_frame_view.h
[add] https://crrev.com/3f9933ea300cdd3bc77e927c2328f6ce55468bef/chrome/browser/ui/views/frame/glass_browser_frame_view_browsertest_win.cc
[modify] https://crrev.com/3f9933ea300cdd3bc77e927c2328f6ce55468bef/chrome/browser/ui/views/frame/hosted_app_button_container.cc
[modify] https://crrev.com/3f9933ea300cdd3bc77e927c2328f6ce55468bef/chrome/browser/ui/views/frame/hosted_app_button_container.h
[modify] https://crrev.com/3f9933ea300cdd3bc77e927c2328f6ce55468bef/chrome/test/BUILD.gn

WIP screencast of origin text slide.
Video-Fri-Jul-13-2018-15-45-44.webm
7.7 MB View Download
Cc: austinknight@chromium.org
+austinknight: FYI the WIP behaviour of the origin slide ovelapping the page title.
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 24

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

commit f09b898143cb3102ac468d4f968d2a385659c05b
Author: Alan Cutter <alancutter@chromium.org>
Date: Tue Jul 24 07:21:02 2018

Refactor FrameHeaderOriginText to be reusable by GlassBrowserFrameView

This CL:
 - Renames FrameHeaderOriginText to HostedAppOriginText as it's not
   intended to be used for anything other than hosted app windows
   and for consistency with other HostedApp* views.
 - Moves HostedAppOriginText from ash/frame to
   chrome/browser/ui/views/frame.
 - Moves the animation timing logic from BrowserNonClientFrameViewAsh
   into HostedAppButtonContainer.

This is in preparation to add the origin text animation to
GlassBrowserFrameView (see https://chromium-review.googlesource.com/c/chromium/src/+/1136270).
There are no behavioural changes in this CL.

Bug:  854479 
Change-Id: I69934e5b4595356d10e707011a231f9c095338db
Reviewed-on: https://chromium-review.googlesource.com/1134713
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577445}
[modify] https://crrev.com/f09b898143cb3102ac468d4f968d2a385659c05b/ash/BUILD.gn
[modify] https://crrev.com/f09b898143cb3102ac468d4f968d2a385659c05b/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/f09b898143cb3102ac468d4f968d2a385659c05b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/f09b898143cb3102ac468d4f968d2a385659c05b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
[modify] https://crrev.com/f09b898143cb3102ac468d4f968d2a385659c05b/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
[modify] https://crrev.com/f09b898143cb3102ac468d4f968d2a385659c05b/chrome/browser/ui/views/frame/hosted_app_button_container.cc
[modify] https://crrev.com/f09b898143cb3102ac468d4f968d2a385659c05b/chrome/browser/ui/views/frame/hosted_app_button_container.h
[rename] https://crrev.com/f09b898143cb3102ac468d4f968d2a385659c05b/chrome/browser/ui/views/frame/hosted_app_origin_text.cc
[rename] https://crrev.com/f09b898143cb3102ac468d4f968d2a385659c05b/chrome/browser/ui/views/frame/hosted_app_origin_text.h
[modify] https://crrev.com/f09b898143cb3102ac468d4f968d2a385659c05b/chrome/browser/ui/views/frame/immersive_mode_controller_ash_browsertest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 24

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

commit f95b60e3b417559186c8e4e06a5233549d6fa014
Author: Alan Cutter <alancutter@chromium.org>
Date: Tue Jul 24 23:44:09 2018

Add origin text slide animation to Windows hosted app titlebars

This CL adds a small origin text slide animation for Windows
hosted app titlebars. This brings Windows closer to UI parity
with ChromeOS.

This change is hidden behind the DesktopPWAWindowing feature flag.

Screencast:
https://bugs.chromium.org/p/chromium/issues/attachment?aid=348231&signed_aid=gIhJQdNaKHk_-6Ujfl2XxA==&inline=1

Bug:  854479 
Change-Id: I848cab575dc4efa3001e5101b4d4795818a8369f
Reviewed-on: https://chromium-review.googlesource.com/1136270
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577732}
[modify] https://crrev.com/f95b60e3b417559186c8e4e06a5233549d6fa014/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
[modify] https://crrev.com/f95b60e3b417559186c8e4e06a5233549d6fa014/chrome/browser/ui/views/frame/glass_browser_frame_view.h
[modify] https://crrev.com/f95b60e3b417559186c8e4e06a5233549d6fa014/chrome/browser/ui/views/frame/hosted_app_origin_text.cc
[modify] https://crrev.com/f95b60e3b417559186c8e4e06a5233549d6fa014/chrome/browser/ui/views/frame/hosted_app_origin_text.h

Summary: Bring Windows PWA window frames up to feature parity with ChromeOS (was: Bring Windows 10 PWA window frames up to feature parity with ChromeOS)
Bringing Windows 7 in scope of this bug as we intend to add support for it at the same time.
WIP CL: https://chromium-review.googlesource.com/c/chromium/src/+/1152718
Screenshots for refactoring HostedAppOriginText into HostedAppButtonContainer to make it easier (and less copy pastey) to add to OpaqueBrowserFrameView.
chromeos-before.png
54.9 KB View Download
chromeos-after.png
54.7 KB View Download
chromeos-mash-before.png
55.8 KB View Download
chromeos-mash-after.png
54.7 KB View Download
windows10-before.png
8.1 KB View Download
windows10-after.png
8.1 KB View Download
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 30

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

commit dd0ee5d6c3c38be7faa7a2b4eeede83270770e19
Author: Alan Cutter <alancutter@chromium.org>
Date: Mon Jul 30 02:16:01 2018

Force OpaqueBrowserFrameView for hosted apps on Windows 7

This CL forces Windows 7 to use the OpaqueBrowserFrameView (instead of
GlassBrowserFrameView) for hosted app windows. This is in preparation to
consistently support extra hosted app controls in the title bar before
we have implemented support for Aero glass titlebars.

This change is hidden behind the DesktopPWAWindowing flag.

Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=350562&signed_aid=YMSTIPY7Xl2vwJUHiVXzPw==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=350563&signed_aid=XwsCSHyIFeGdrm_6R_PkVQ==&inline=1

Bug:  854479 
Change-Id: I3232648004ff7b32657afa127fe76924aa4f2950
Reviewed-on: https://chromium-review.googlesource.com/1152722
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578965}
[modify] https://crrev.com/dd0ee5d6c3c38be7faa7a2b4eeede83270770e19/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc
[modify] https://crrev.com/dd0ee5d6c3c38be7faa7a2b4eeede83270770e19/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.h

Project Member

Comment 14 by bugdroid1@chromium.org, Jul 30

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

commit 85d4706dfd9ab0daee04349c2e6400019b312f21
Author: Alan Cutter <alancutter@chromium.org>
Date: Mon Jul 30 06:27:30 2018

Move HostedAppOriginText into HostedAppButtonContainer

This CL refactors HostedAppOriginText to be a child view of HostedAppButtonContainer.
This reduces the amount of copy paste code required for BrowserNonClientFrameViews to
include hosted app titlebar controls.
There are no behavioural changes in this patch.

This is in preparation to add HostedAppButtonContainer to OpaqueBrowserFrameView as
part of  crbug.com/854479 .

ChromeOS:
Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=350571&signed_aid=YOEpL6GXAWvEz_Z3y_oEGQ==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=350572&signed_aid=OPqcFEyjoPZPeTeV_gOCVw==&inline=1

ChromeOS Mash:
Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=350573&signed_aid=XZV7AbxDTBXdKws05j49uw==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=350574&signed_aid=afuv2dBuAZOzE5KMlE5x-A==&inline=1

Windows 10:
Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=350575&signed_aid=foMVbIlc5-EyYC_3xWeLIw==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=350576&signed_aid=-lTiU3fJfkAr_3c_z2Tu3g==&inline=1

Bug:  854479 
Change-Id: I275af3c8079a895d23dec1d56230648ca1784aae
Reviewed-on: https://chromium-review.googlesource.com/1152718
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578984}
[modify] https://crrev.com/85d4706dfd9ab0daee04349c2e6400019b312f21/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/85d4706dfd9ab0daee04349c2e6400019b312f21/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
[modify] https://crrev.com/85d4706dfd9ab0daee04349c2e6400019b312f21/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
[modify] https://crrev.com/85d4706dfd9ab0daee04349c2e6400019b312f21/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
[modify] https://crrev.com/85d4706dfd9ab0daee04349c2e6400019b312f21/chrome/browser/ui/views/frame/glass_browser_frame_view.h
[modify] https://crrev.com/85d4706dfd9ab0daee04349c2e6400019b312f21/chrome/browser/ui/views/frame/hosted_app_button_container.cc
[modify] https://crrev.com/85d4706dfd9ab0daee04349c2e6400019b312f21/chrome/browser/ui/views/frame/hosted_app_button_container.h
[modify] https://crrev.com/85d4706dfd9ab0daee04349c2e6400019b312f21/chrome/browser/ui/views/frame/hosted_app_origin_text.cc
[modify] https://crrev.com/85d4706dfd9ab0daee04349c2e6400019b312f21/chrome/browser/ui/views/frame/hosted_app_origin_text.h

WIP screenshots of Windows 7 PWA controls:
https://chromium-review.googlesource.com/c/chromium/src/+/1156117
no-theme.png
25.2 KB View Download
light-theme.png
17.6 KB View Download
dark-theme.png
10.4 KB View Download
Project Member

Comment 16 by bugdroid1@chromium.org, Aug 2

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

commit 2634272e0f5fb763bcb5ec7c5e4aa7e24a6e0d13
Author: Alan Cutter <alancutter@chromium.org>
Date: Thu Aug 02 03:16:21 2018

Refactor coordinate system of OpaqueBrowserFrameViewLayout::trailing_button_start_

This CL is a refactor and has no behavioural changes.

OpaqueBrowserFrameView::trailing_button_start_ is an x coordinate
relative to the right edge of the window title bar.

This CL replaces trailing_button_start_ with available_space_trailing_x_
where
available_space_trailing_x_ == host->width() - trailing_button_start_.

available_space_trailing_x_ represents the same x position as
trailing_button_start_ except it's relative to the left edge of the
titlebar (0).

The name "button_start" has been replaced with "available_space_*_x"
as it's used for more than just button elements and now represents
a typical x co-ordinate.

This refactor is intended to support
https://chromium-review.googlesource.com/c/chromium/src/+/1156117
by making HostedAppButtonContainer::LayoutInContainer() less cumbersome
to use in OpaqueBrowserFrameViewLayout.

Bug:  854479 
Change-Id: Iff1c29139cd4c8c66286db85fce85b99c94c5261
Reviewed-on: https://chromium-review.googlesource.com/1158313
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580045}
[modify] https://crrev.com/2634272e0f5fb763bcb5ec7c5e4aa7e24a6e0d13/chrome/browser/ui/views/frame/desktop_linux_browser_frame_view_layout.cc
[modify] https://crrev.com/2634272e0f5fb763bcb5ec7c5e4aa7e24a6e0d13/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.cc
[modify] https://crrev.com/2634272e0f5fb763bcb5ec7c5e4aa7e24a6e0d13/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.h

Project Member

Comment 17 by bugdroid1@chromium.org, Aug 3

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

commit e70a6109f43e2d171b21666258aad6808685331f
Author: Alan Cutter <alancutter@chromium.org>
Date: Fri Aug 03 02:57:52 2018

Add hosted app window controls to Windows 7 hosted app windows

This CL adds the HostedAppButtonContainer to OpaqueBrowserFrameView
for hosted app (including PWA) windows.
This brings hosted app windows in line with Windows 10 and is in
preparation to do the same for Linux.

This change is hidden behind the DesktopPWAWindowing flag.

No theme color: https://bugs.chromium.org/p/chromium/issues/attachment?aid=351013&signed_aid=DKxP-apBa2Tk8wt4cMG6Lw==&inline=1
Light theme color: https://bugs.chromium.org/p/chromium/issues/attachment?aid=351014&signed_aid=WvEQ9H_tFU9xhHODR8SOrA==&inline=1
Dark theme color: https://bugs.chromium.org/p/chromium/issues/attachment?aid=351015&signed_aid=C7Y4ZEgoFwKF5ECOmFVHEA==&inline=1

Bug:  854479 
Change-Id: I058bcd139532ddcbea223a6cb1a391d13a16e8ab
Reviewed-on: https://chromium-review.googlesource.com/1156117
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580420}
[modify] https://crrev.com/e70a6109f43e2d171b21666258aad6808685331f/chrome/browser/ui/view_ids.h
[modify] https://crrev.com/e70a6109f43e2d171b21666258aad6808685331f/chrome/browser/ui/views/frame/hosted_app_button_container.cc
[modify] https://crrev.com/e70a6109f43e2d171b21666258aad6808685331f/chrome/browser/ui/views/frame/hosted_app_button_container.h
[modify] https://crrev.com/e70a6109f43e2d171b21666258aad6808685331f/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
[modify] https://crrev.com/e70a6109f43e2d171b21666258aad6808685331f/chrome/browser/ui/views/frame/opaque_browser_frame_view.h
[modify] https://crrev.com/e70a6109f43e2d171b21666258aad6808685331f/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.cc
[modify] https://crrev.com/e70a6109f43e2d171b21666258aad6808685331f/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.h

Blocking: -851845
Labels: M-70
Status: Fixed (was: Started)
The only thing missing now is the conditional back button which is still under consideration and not blocking M70.

Sign in to add a comment