Skip to content

Commit

Permalink
[bookmarks] Prevent ListMenuButtonDelegate setter from setting visibi…
Browse files Browse the repository at this point in the history
…lity

When the ListMenuButtonDelegate was set, the menu was set to visible
immediately. This conflicts with the direct visibility setter
BOOKMARK_EDITABLE, which causes ordering issues depending on which
property was set first. Changing that so the only property that
affects list menu visibility is BOOKMARK_EDITABLE.

Bug: 1447549
Change-Id: I615ccdebf35753aca72ebfea27e757ff2f8b293e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4553699
Reviewed-by: Sky Malice <skym@chromium.org>
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1148114}
  • Loading branch information
Brandon Wylie authored and Chromium LUCI CQ committed May 23, 2023
1 parent 1b8dc82 commit 1d71a33
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ void setAccessoryView(@Nullable View view) {

void setListMenuButtonDelegate(ListMenuButtonDelegate listMenuButtonDelegate) {
mMoreButton.setDelegate(listMenuButtonDelegate);
mMoreButton.setVisibility(View.VISIBLE);
}

void setPopupListener(PopupMenuShownListener listener) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,4 +141,13 @@ public void testSelectionInactive() {
Assert.assertEquals(View.IMPORTANT_FOR_ACCESSIBILITY_YES,
mImprovedBookmarkRow.findViewById(R.id.more).getImportantForAccessibility());
}

@Test
public void testListMenuButtonDelegateDoesNotChangeVisibility() {
int visibility = mImprovedBookmarkRow.findViewById(R.id.more).getVisibility();
mModel.set(ImprovedBookmarkRowProperties.LIST_MENU_BUTTON_DELEGATE, null);
// Setting the delegate shouldn't affect visibility.
Assert.assertEquals(
visibility, mImprovedBookmarkRow.findViewById(R.id.more).getVisibility());
}
}

0 comments on commit 1d71a33

Please sign in to comment.