-
Notifications
You must be signed in to change notification settings - Fork 6.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
sharing: implement desktop share preview stub
This change adds the preview surface to the desktop sharing hub, gated by the DesktopSharePreview field trial (or UpcomingSharingFeatures, as usual). Specifically, this change: 1. Adds methods to SharingHubBubbleController to get the needed information about the pending share - these are currently stubs that operate on the active WebContents only, for test purposes (see https://crbug.com/1312524) 2. Adds a new class PreviewView which displays that information 3. Adds code to SharingHubBubbleViewImpl to construct a PreviewView and insert it into its layout when the feature is enabled A couple of design things: * PreviewView is fully decoupled from both SharingHubBubbleController and SharingHubBubbleViewImpl. SharingHubBubbleViewImpl is responsible for registering PreviewView for image update callbacks, so PreviewView has the ability to hold a provided CallbackListSubscription, but it is unaware of the other two involved classes. This should make it quite easy to unit test. * I considered creating a wrapper type to hold the share preview state, but ultimately decided against it; it led to a design where PreviewView (appeared to) allow updates of all parts of its visuals, not just the image, which is more flexibility than I wanted to add. Next steps: * Unit tests for PreviewView, once it has more logic in it * Wiring SharingHubBubbleController to the actual omnibox share state * Metrics gathering * Working with UX to reconcile the differences from the mock redlines Fixed: 1312516 Change-Id: If55a23cfee64dbaa156d6a257d3f0da1a37bcc3d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3571755 Reviewed-by: Kristi Park <kristipark@chromium.org> Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org> Cr-Commit-Position: refs/heads/main@{#989609}
- Loading branch information
Elly Fong-Jones
authored and
Chromium LUCI CQ
committed
Apr 6, 2022
1 parent
8b0b695
commit eb519b0
Showing
7 changed files
with
191 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| // Copyright 2022 The Chromium Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #include "chrome/browser/ui/views/sharing_hub/preview_view.h" | ||
|
|
||
| #include "base/strings/utf_string_conversions.h" | ||
| #include "chrome/browser/ui/sharing_hub/sharing_hub_bubble_controller.h" | ||
| #include "ui/views/controls/image_view.h" | ||
| #include "ui/views/controls/label.h" | ||
| #include "ui/views/layout/flex_layout.h" | ||
| #include "ui/views/view_class_properties.h" | ||
|
|
||
| namespace sharing_hub { | ||
|
|
||
| namespace { | ||
| constexpr gfx::Size kImageSize{32, 32}; | ||
| } // namespace | ||
|
|
||
| // This view uses two nested FlexLayouts, a horizontal outer one and a vertical | ||
| // inner one, to create a composite layout where the icon is aligned with the | ||
| // title and URL taken together. The resulting View tree looks like this: | ||
| // PreviewView [FlexLayout, horizontal] | ||
| // ImageView | ||
| // View [FlexLayout, vertical] | ||
| // Label (title) | ||
| // Label (URL) | ||
| PreviewView::PreviewView(std::u16string title, GURL url, ui::ImageModel image) { | ||
| auto* layout = SetLayoutManager(std::make_unique<views::FlexLayout>()); | ||
| layout->SetOrientation(views::LayoutOrientation::kHorizontal) | ||
| .SetMainAxisAlignment(views::LayoutAlignment::kStart) | ||
| .SetCrossAxisAlignment(views::LayoutAlignment::kCenter) | ||
| // TODO(ellyjones): Check these against the redlines in the mocks, and | ||
| // once they're finalized, move them to the constant block above. Getting | ||
| // things to line up with the mocks may require a bit of a refactor in | ||
| // HoverButton itself. | ||
| .SetInteriorMargin(gfx::Insets::VH(12, 8)) | ||
| .SetDefault(views::kMarginsKey, gfx::Insets::VH(0, 8)); | ||
|
|
||
| image_ = AddChildView(std::make_unique<views::ImageView>(image)); | ||
| image_->SetPreferredSize(kImageSize); | ||
|
|
||
| auto* labels_container = AddChildView(std::make_unique<views::View>()); | ||
| auto* labels_layout = | ||
| labels_container->SetLayoutManager(std::make_unique<views::FlexLayout>()); | ||
| labels_layout->SetOrientation(views::LayoutOrientation::kVertical) | ||
| .SetMainAxisAlignment(views::LayoutAlignment::kCenter) | ||
| .SetCrossAxisAlignment(views::LayoutAlignment::kStart); | ||
|
|
||
| // TODO(ellyjones): These do not exactly match the redlines, which call for | ||
| // 14pt Roboto specifically. We should probably update the redlines to not | ||
| // use a hardcoded font, but we could also specify the font more explicitly | ||
| // here. | ||
| title_ = labels_container->AddChildView(std::make_unique<views::Label>( | ||
| title, views::style::CONTEXT_DIALOG_TITLE)); | ||
| url_ = labels_container->AddChildView(std::make_unique<views::Label>( | ||
| base::UTF8ToUTF16(url.spec()), views::style::CONTEXT_DIALOG_TITLE, | ||
| views::style::STYLE_HINT)); | ||
| } | ||
|
|
||
| PreviewView::~PreviewView() = default; | ||
|
|
||
| void PreviewView::TakeCallbackSubscription( | ||
| base::CallbackListSubscription subscription) { | ||
| subscription_ = std::move(subscription); | ||
| } | ||
|
|
||
| void PreviewView::OnImageChanged(ui::ImageModel model) { | ||
| image_->SetImage(model); | ||
| } | ||
|
|
||
| } // namespace sharing_hub |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| // Copyright 2022 The Chromium Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #ifndef CHROME_BROWSER_UI_VIEWS_SHARING_HUB_PREVIEW_VIEW_H_ | ||
| #define CHROME_BROWSER_UI_VIEWS_SHARING_HUB_PREVIEW_VIEW_H_ | ||
|
|
||
| #include "ui/base/models/image_model.h" | ||
| #include "ui/views/view.h" | ||
| #include "url/gurl.h" | ||
|
|
||
| namespace views { | ||
| class ImageView; | ||
| class Label; | ||
| } // namespace views | ||
|
|
||
| namespace sharing_hub { | ||
|
|
||
| // A PreviewView shows some information about a pending share so the user can | ||
| // tell what they're about to share. In particular, it looks like this: | ||
| // +-----------------------------+ | ||
| // |+------+ Title | | ||
| // || Icon | | | ||
| // |+------+ URL | | ||
| // +-----------------------------+ | ||
| // The title and URL are fixed at construction time, but the icon may change - | ||
| // which image is used depends on the state of the DesktopSharePreview field | ||
| // trial. | ||
| class PreviewView : public views::View { | ||
| public: | ||
| // Taking an initial image here, instead of requiring the caller to call | ||
| // OnImageChanged() after construction to set the initial image, means that | ||
| // this class always has a valid image to display and does not have a | ||
| // "half-initialized" state to worry about. | ||
| explicit PreviewView(std::u16string title, GURL url, ui::ImageModel image); | ||
| ~PreviewView() override; | ||
|
|
||
| // This seemingly-odd method allows for PreviewView to be uncoupled from the | ||
| // class that provides image updates - it receives image updates via a | ||
| // callback which is bound by external code. Having PreviewView itself store | ||
| // the subscription guarantees that the callback can't be delivered on a | ||
| // deleted PreviewView. | ||
| void TakeCallbackSubscription(base::CallbackListSubscription subscription); | ||
|
|
||
| // Call this method to supply a new ImageModel to use for the preview image. | ||
| // Whatever image you supply will be scaled to fit the image slot. | ||
| void OnImageChanged(ui::ImageModel model); | ||
|
|
||
| private: | ||
| base::CallbackListSubscription subscription_; | ||
|
|
||
| views::Label* title_ = nullptr; | ||
| views::Label* url_ = nullptr; | ||
| views::ImageView* image_ = nullptr; | ||
| }; | ||
|
|
||
| } // namespace sharing_hub | ||
|
|
||
| #endif // CHROME_BROWSER_UI_VIEWS_SHARING_HUB_PREVIEW_VIEW_H_ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters