0

Use PickerView width to determine section grid widths.

Pass PickerView width to PickerSectionView so that the sections can
determine how grid items should be laid out. (This is needed to
determine when new rows should be created when adding small grid items
and to resize images to fit into an image grid.)

Bug: b:323279115
Change-Id: I46beef499ff299f871d10b9ebd630ffce8ae21b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5273784
Commit-Queue: Michelle Chen <michellegc@google.com>
Reviewed-by: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1257234}
This commit is contained in:
Michelle
2024-02-07 08:26:56 +00:00
committed by Chromium LUCI CQ
parent bc0f0c74be
commit 7ed2289459
12 changed files with 71 additions and 50 deletions

@ -17,6 +17,7 @@
namespace ash {
PickerCategoryView::PickerCategoryView(
int picker_view_width,
PickerSearchResultsView::SelectSearchResultCallback
select_search_result_callback,
PickerAssetFetcher* asset_fetcher) {
@ -24,7 +25,8 @@ PickerCategoryView::PickerCategoryView(
->SetOrientation(views::LayoutOrientation::kVertical);
search_results_view_ = AddChildView(std::make_unique<PickerSearchResultsView>(
std::move(select_search_result_callback), asset_fetcher));
picker_view_width, std::move(select_search_result_callback),
asset_fetcher));
}
PickerCategoryView::~PickerCategoryView() = default;

@ -21,6 +21,7 @@ class ASH_EXPORT PickerCategoryView : public views::View {
public:
explicit PickerCategoryView(
int picker_view_width,
PickerSearchResultsView::SelectSearchResultCallback
select_search_result_callback,
PickerAssetFetcher* asset_fetcher);

@ -32,9 +32,11 @@
namespace ash {
PickerSearchResultsView::PickerSearchResultsView(
int picker_view_width,
SelectSearchResultCallback select_search_result_callback,
PickerAssetFetcher* asset_fetcher)
: select_search_result_callback_(std::move(select_search_result_callback)),
: picker_view_width_(picker_view_width),
select_search_result_callback_(std::move(select_search_result_callback)),
asset_fetcher_(asset_fetcher) {
SetLayoutManager(std::make_unique<views::FlexLayout>())
->SetOrientation(views::LayoutOrientation::kVertical);
@ -49,8 +51,8 @@ void PickerSearchResultsView::SetSearchResults(
section_views_.clear();
RemoveAllChildViews();
for (const auto& section : search_results_.sections()) {
auto* section_view =
AddChildView(std::make_unique<PickerSectionView>(section.heading()));
auto* section_view = AddChildView(std::make_unique<PickerSectionView>(
picker_view_width_, section.heading()));
for (const auto& result : section.results()) {
AddResultToSection(result, section_view);
}

@ -29,6 +29,7 @@ class ASH_EXPORT PickerSearchResultsView : public views::View {
// `asset_fetcher` must remain valid for the lifetime of this class.
explicit PickerSearchResultsView(
int picker_view_width,
SelectSearchResultCallback select_search_result_callback,
PickerAssetFetcher* asset_fetcher);
PickerSearchResultsView(const PickerSearchResultsView&) = delete;
@ -53,6 +54,9 @@ class ASH_EXPORT PickerSearchResultsView : public views::View {
void AddResultToSection(const PickerSearchResult& result,
PickerSectionView* section_view);
// Width of the containing PickerView.
int picker_view_width_ = 0;
SelectSearchResultCallback select_search_result_callback_;
PickerSearchResults search_results_;

@ -30,6 +30,8 @@ using ::testing::Pointee;
using ::testing::Property;
using ::testing::SizeIs;
constexpr int kPickerWidth = 320;
using PickerSearchResultsViewTest = AshTestBase;
auto MatchesResultSection(const PickerSearchResults::Section& section) {
@ -42,7 +44,7 @@ auto MatchesResultSection(const PickerSearchResults::Section& section) {
TEST_F(PickerSearchResultsViewTest, CreatesResultsSections) {
MockPickerAssetFetcher asset_fetcher;
PickerSearchResultsView view(base::DoNothing(), &asset_fetcher);
PickerSearchResultsView view(kPickerWidth, base::DoNothing(), &asset_fetcher);
const PickerSearchResults kSearchResults({{
PickerSearchResults::Section(u"Section 1",
{{PickerSearchResult::Text(u"Result A")}}),
@ -61,7 +63,7 @@ TEST_F(PickerSearchResultsViewTest, CreatesResultsSections) {
TEST_F(PickerSearchResultsViewTest, CreatesResultsSectionWithGif) {
MockPickerAssetFetcher asset_fetcher;
PickerSearchResultsView view(base::DoNothing(), &asset_fetcher);
PickerSearchResultsView view(kPickerWidth, base::DoNothing(), &asset_fetcher);
const PickerSearchResults kSearchResults({{PickerSearchResults::Section(
u"Gif Section", {{PickerSearchResult::Gif(GURL(), gfx::Size())}})}});
view.SetSearchResults(kSearchResults);
@ -74,7 +76,7 @@ TEST_F(PickerSearchResultsViewTest, CreatesResultsSectionWithGif) {
TEST_F(PickerSearchResultsViewTest, UpdatesResultsSections) {
MockPickerAssetFetcher asset_fetcher;
PickerSearchResultsView view(base::DoNothing(), &asset_fetcher);
PickerSearchResultsView view(kPickerWidth, base::DoNothing(), &asset_fetcher);
const PickerSearchResults kInitialSearchResults({{
PickerSearchResults::Section(u"Section",
{{PickerSearchResult::Text(u"Result")}}),
@ -110,7 +112,7 @@ TEST_P(PickerSearchResultsViewResultSelectionTest, LeftClickSelectsResult) {
MockPickerAssetFetcher asset_fetcher;
auto* view =
widget->SetContentsView(std::make_unique<PickerSearchResultsView>(
future.GetCallback(), &asset_fetcher));
kPickerWidth, future.GetCallback(), &asset_fetcher));
view->SetSearchResults(PickerSearchResults({{
PickerSearchResults::Section(u"section", {{test_case.result}}),
}}));

@ -48,8 +48,13 @@ constexpr gfx::Size kSmallGridItemPreferredSize(32, 32);
// Padding between and around image grid items.
constexpr int kImageGridPadding = 8;
// TODO: b/323279115 - Compute this in terms of available width.
constexpr int kImageGridColumnWidth = 148;
// Number of columns in an image grid.
constexpr int kNumImageGridColumns = 2;
int GetImageGridColumnWidth(int section_width) {
return (section_width - (kNumImageGridColumns + 1) * kImageGridPadding) /
kNumImageGridColumns;
}
std::unique_ptr<views::View> CreateSmallItemsGridRow() {
auto row = views::Builder<views::FlexLayoutView>()
@ -114,7 +119,9 @@ std::unique_ptr<views::View> CreateListItemsContainer() {
} // namespace
PickerSectionView::PickerSectionView(const std::u16string& title_text) {
PickerSectionView::PickerSectionView(int section_width,
const std::u16string& title_text)
: section_width_(section_width) {
SetLayoutManager(std::make_unique<views::FlexLayout>())
->SetOrientation(views::LayoutOrientation::kVertical);
@ -127,10 +134,6 @@ PickerSectionView::PickerSectionView(const std::u16string& title_text) {
PickerSectionView::~PickerSectionView() = default;
void PickerSectionView::SetMaximumWidth(int maximum_width) {
maximum_width_ = maximum_width;
}
void PickerSectionView::AddListItem(std::unique_ptr<views::View> list_item) {
if (list_items_container_ == nullptr) {
list_items_container_ = AddChildView(CreateListItemsContainer());
@ -166,7 +169,7 @@ void PickerSectionView::AddImageItem(
image_grid_ = AddChildView(CreateImageGrid());
}
image_item->SetImageSizeFromWidth(kImageGridColumnWidth);
image_item->SetImageSizeFromWidth(GetImageGridColumnWidth(section_width_));
views::View* shortest_column =
base::ranges::min(image_grid_->children(),
/*comp=*/base::ranges::less(),
@ -186,10 +189,10 @@ void PickerSectionView::AddSmallGridItem(
// Try to add the item to the last row. If it doesn't fit, create a new row
// and add the item there.
views::View* row = small_items_grid_->children().back();
if (!row->children().empty() && maximum_width_.has_value() &&
if (!row->children().empty() &&
row->GetPreferredSize().width() + kSmallGridItemMargins.left() +
grid_item->GetPreferredSize().width() >
maximum_width_.value()) {
section_width_) {
row = small_items_grid_->AddChildView(CreateSmallItemsGridRow());
}
item_views_.push_back(row->AddChildView(std::move(grid_item)));

@ -31,14 +31,12 @@ class ASH_EXPORT PickerSectionView : public views::View {
METADATA_HEADER(PickerSectionView, views::View)
public:
explicit PickerSectionView(const std::u16string& title_text);
explicit PickerSectionView(int section_width,
const std::u16string& title_text);
PickerSectionView(const PickerSectionView&) = delete;
PickerSectionView& operator=(const PickerSectionView&) = delete;
~PickerSectionView() override;
// Sets the maximum width available for laying out section items.
void SetMaximumWidth(int maximum_width);
// Adds a list item. These are displayed in a vertical list, each item
// spanning the width of the section.
void AddListItem(std::unique_ptr<views::View> list_item);
@ -66,15 +64,12 @@ class ASH_EXPORT PickerSectionView : public views::View {
}
private:
// Adds a small grid item. These are displayed in rows. If there may be more
// than one row, `maximum_width_` should be set before adding small grid items
// to ensure the rows are laid out correctly.
// Adds a small grid item. These are displayed in rows.
void AddSmallGridItem(std::unique_ptr<views::View> small_grid_item);
// Maximum width available for laying out section items. If not set, we assume
// the available width is unbounded during layout, so small grid items will be
// laid out in a single row.
std::optional<int> maximum_width_;
// Width available for laying out section items. This is needed to determine
// row and column widths for grid items in the section.
int section_width_ = 0;
raw_ptr<views::Label> title_ = nullptr;

@ -27,6 +27,8 @@ using ::testing::Pointee;
using ::testing::Property;
using ::testing::SizeIs;
constexpr int kDefaultSectionWidth = 320;
int GetAspectRatio(const gfx::Size& size) {
return size.height() / size.width();
}
@ -49,7 +51,7 @@ std::unique_ptr<PickerImageItemView> CreateGifItem(
using PickerSectionViewTest = AshTestBase;
TEST_F(PickerSectionViewTest, AddsEmojiItem) {
PickerSectionView section_view(u"Section");
PickerSectionView section_view(kDefaultSectionWidth, u"Section");
section_view.AddEmojiItem(std::make_unique<PickerEmojiItemView>(
views::Button::PressedCallback(), u"😊"));
@ -61,7 +63,7 @@ TEST_F(PickerSectionViewTest, AddsEmojiItem) {
}
TEST_F(PickerSectionViewTest, AddsSymbolItem) {
PickerSectionView section_view(u"Section");
PickerSectionView section_view(kDefaultSectionWidth, u"Section");
section_view.AddSymbolItem(std::make_unique<PickerSymbolItemView>(
views::Button::PressedCallback(), u""));
@ -73,7 +75,7 @@ TEST_F(PickerSectionViewTest, AddsSymbolItem) {
}
TEST_F(PickerSectionViewTest, AddsEmoticonItem) {
PickerSectionView section_view(u"Section");
PickerSectionView section_view(kDefaultSectionWidth, u"Section");
section_view.AddEmoticonItem(std::make_unique<PickerEmoticonItemView>(
views::Button::PressedCallback(), u"¯\\_(ツ)_/¯"));
@ -84,10 +86,9 @@ TEST_F(PickerSectionViewTest, AddsEmoticonItem) {
ElementsAre(Pointee(Property(&views::View::children, SizeIs(1)))));
}
TEST_F(PickerSectionViewTest, SmallGridItemsStayWithinMaximumWidth) {
PickerSectionView section_view(u"Section");
TEST_F(PickerSectionViewTest, SmallGridItemsStayWithinSectionWidth) {
PickerSectionView section_view(kDefaultSectionWidth, u"Section");
section_view.SetMaximumWidth(320);
section_view.AddEmoticonItem(CreateSizedEmoticonItem(gfx::Size(100, 40)));
section_view.AddEmoticonItem(CreateSizedEmoticonItem(gfx::Size(80, 40)));
section_view.AddEmoticonItem(CreateSizedEmoticonItem(gfx::Size(90, 40)));
@ -101,7 +102,7 @@ TEST_F(PickerSectionViewTest, SmallGridItemsStayWithinMaximumWidth) {
}
TEST_F(PickerSectionViewTest, OneGifItem) {
PickerSectionView section_view(u"Section");
PickerSectionView section_view(kDefaultSectionWidth, u"Section");
section_view.AddImageItem(CreateGifItem(gfx::Size(100, 100)));
@ -113,7 +114,7 @@ TEST_F(PickerSectionViewTest, OneGifItem) {
}
TEST_F(PickerSectionViewTest, TwoGifItems) {
PickerSectionView section_view(u"Section");
PickerSectionView section_view(kDefaultSectionWidth, u"Section");
section_view.AddImageItem(CreateGifItem(gfx::Size(100, 100)));
section_view.AddImageItem(CreateGifItem(gfx::Size(100, 100)));
@ -126,7 +127,7 @@ TEST_F(PickerSectionViewTest, TwoGifItems) {
}
TEST_F(PickerSectionViewTest, GifItemsWithVaryingHeight) {
PickerSectionView section_view(u"Section");
PickerSectionView section_view(kDefaultSectionWidth, u"Section");
section_view.AddImageItem(CreateGifItem(gfx::Size(100, 120)));
section_view.AddImageItem(CreateGifItem(gfx::Size(100, 20)));
@ -141,7 +142,7 @@ TEST_F(PickerSectionViewTest, GifItemsWithVaryingHeight) {
}
TEST_F(PickerSectionViewTest, GifItemsAreResizedToSameWidth) {
PickerSectionView section_view(u"Section");
PickerSectionView section_view(kDefaultSectionWidth, u"Section");
section_view.AddImageItem(CreateGifItem(gfx::Size(100, 100)));
section_view.AddImageItem(CreateGifItem(gfx::Size(80, 160)));
@ -157,7 +158,7 @@ TEST_F(PickerSectionViewTest, GifItemsAreResizedToSameWidth) {
}
TEST_F(PickerSectionViewTest, PreservesAspectRatioOfGifItems) {
PickerSectionView section_view(u"Section");
PickerSectionView section_view(kDefaultSectionWidth, u"Section");
constexpr gfx::Size kGifDimensions(100, 200);
section_view.AddImageItem(CreateGifItem(kGifDimensions));
@ -173,7 +174,7 @@ TEST_F(PickerSectionViewTest, PreservesAspectRatioOfGifItems) {
}
TEST_F(PickerSectionViewTest, EmojiItemsAndGifItems) {
PickerSectionView section_view(u"Section");
PickerSectionView section_view(kDefaultSectionWidth, u"Section");
section_view.AddEmojiItem(std::make_unique<PickerEmojiItemView>(
views::Button::PressedCallback(), u"😊"));

@ -317,14 +317,17 @@ void PickerView::AddContentsView(PickerLayoutType layout_type) {
// `base::Unretained` is safe here because this class owns
// `zero_state_view_`, `category_view_` and `search_results_view`_.
zero_state_view_ = contents_view_->AddPage(
std::make_unique<PickerZeroStateView>(base::BindRepeating(
&PickerView::SelectCategory, base::Unretained(this))));
zero_state_view_ =
contents_view_->AddPage(std::make_unique<PickerZeroStateView>(
kPickerSize.width(), base::BindRepeating(&PickerView::SelectCategory,
base::Unretained(this))));
category_view_ = contents_view_->AddPage(std::make_unique<PickerCategoryView>(
kPickerSize.width(),
base::BindOnce(&PickerView::SelectSearchResult, base::Unretained(this)),
delegate_->GetAssetFetcher()));
search_results_view_ =
contents_view_->AddPage(std::make_unique<PickerSearchResultsView>(
kPickerSize.width(),
base::BindOnce(&PickerView::SelectSearchResult,
base::Unretained(this)),
delegate_->GetAssetFetcher()));

@ -29,7 +29,9 @@
namespace ash {
PickerZeroStateView::PickerZeroStateView(
SelectCategoryCallback select_category_callback) {
int picker_view_width,
SelectCategoryCallback select_category_callback)
: picker_view_width_(picker_view_width) {
SetLayoutManager(std::make_unique<views::FlexLayout>())
->SetOrientation(views::LayoutOrientation::kVertical);
@ -55,7 +57,7 @@ PickerSectionView* PickerZeroStateView::GetOrCreateSectionView(
}
auto* section_view = AddChildView(std::make_unique<PickerSectionView>(
GetSectionTitleForPickerCategoryType(category_type)));
picker_view_width_, GetSectionTitleForPickerCategoryType(category_type)));
section_views_.insert({category_type, section_view});
return section_view;
}

@ -29,7 +29,8 @@ class ASH_EXPORT PickerZeroStateView : public views::View {
using SelectCategoryCallback =
base::RepeatingCallback<void(PickerCategory category)>;
explicit PickerZeroStateView(SelectCategoryCallback select_category_callback);
explicit PickerZeroStateView(int picker_view_width,
SelectCategoryCallback select_category_callback);
PickerZeroStateView(const PickerZeroStateView&) = delete;
PickerZeroStateView& operator=(const PickerZeroStateView&) = delete;
~PickerZeroStateView() override;
@ -43,6 +44,9 @@ class ASH_EXPORT PickerZeroStateView : public views::View {
// Gets or creates the section to contain `category`.
PickerSectionView* GetOrCreateSectionView(PickerCategory category);
// Width of the containing PickerView.
int picker_view_width_ = 0;
// The views for each section of categories.
std::map<PickerCategoryType, raw_ptr<PickerSectionView>> section_views_;
};

@ -26,10 +26,12 @@ using ::testing::IsEmpty;
using ::testing::Key;
using ::testing::Not;
constexpr int kPickerWidth = 320;
using PickerZeroStateViewTest = AshTestBase;
TEST_F(PickerZeroStateViewTest, CreatesExpressionsSection) {
PickerZeroStateView view(base::DoNothing());
PickerZeroStateView view(kPickerWidth, base::DoNothing());
EXPECT_THAT(view.section_views_for_testing(),
Contains(Key(PickerCategoryType::kExpressions)));
@ -39,8 +41,8 @@ TEST_F(PickerZeroStateViewTest, LeftClickSelectsCategory) {
std::unique_ptr<views::Widget> widget = CreateFramelessTestWidget();
widget->SetFullscreen(true);
base::test::TestFuture<PickerCategory> future;
auto* view = widget->SetContentsView(
std::make_unique<PickerZeroStateView>(future.GetRepeatingCallback()));
auto* view = widget->SetContentsView(std::make_unique<PickerZeroStateView>(
kPickerWidth, future.GetRepeatingCallback()));
ASSERT_THAT(view->section_views_for_testing(),
Contains(Key(PickerCategoryType::kExpressions)));
ASSERT_THAT(view->section_views_for_testing()