[Context-Input] Fix flyout menu positioning on scroll [chromium/src : main]
0 views
Skip to first unread message
James Leung (Gerrit)
8:27 PM (3 hours ago) 8:27 PM
to Wenhu Cheng, Chromium LUCI CQ, Ananya Seelam, chromium...@chromium.org, oshima...@chromium.org
Attention needed from Ananya Seelam and Wenhu Cheng
James Leung voted and added 2 comments![]( "Open in Gerrit")
Votes added by James Leung
| Code-Review | +1 |
2 comments
File chrome/test/data/webui/cr_components/composebox/contextual_action_menu_test.ts
Line 879, Patchset 6 (Latest):
createTabSuggestion({tabId: 1, title: 'Tab 1'}),James Leung . unresolved
put in several new lines
File ui/webui/resources/cr_components/composebox/contextual_action_menu.ts
Line 199, Patchset 6 (Latest):
if (flyout && e.target instanceof Node &&James Leung . unresolved
add comment what this if staement means if possible
Related details
Attention is currently required from:
- Ananya Seelam
- Wenhu Cheng
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Wenhu Cheng (Gerrit)
8:34 PM (3 hours ago) 8:34 PM
to James Leung, Chromium LUCI CQ, Ananya Seelam, chromium...@chromium.org, oshima...@chromium.org
Attention needed from Ananya Seelam
Wenhu Cheng added 2 comments![]( "Open in Gerrit")
File chrome/test/data/webui/cr_components/composebox/contextual_action_menu_test.ts
put in several new lines
Wenhu Cheng
Done
File ui/webui/resources/cr_components/composebox/contextual_action_menu.ts
add comment what this if staement means if possible
Wenhu Cheng
Done
Related details
Attention is currently required from:
- Ananya Seelam
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Wenhu Cheng (Gerrit)
8:34 PM (3 hours ago) 8:34 PM
to James Leung, Chromium LUCI CQ, Ananya Seelam, chromium...@chromium.org, oshima...@chromium.org
Attention needed from Ananya Seelam
Wenhu Cheng voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention is currently required from:
- Ananya Seelam
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
9:20 PM (2 hours ago) 9:20 PM
to Wenhu Cheng, James Leung, Ananya Seelam, chromium...@chromium.org, oshima...@chromium.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
6 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: ui/webui/resources/cr_components/composebox/contextual_action_menu.ts
Insertions: 2, Deletions: 0.
@@ -196,6 +196,8 @@
}
const flyout =
this.shadowRoot?.querySelector<HTMLElement>('.share-tabs-flyout');
+ // Ignore scroll events originating from within the flyout's own content
+ // (e.g. scrolling a long list of tab suggestions).
if (flyout && e.target instanceof Node &&
(e.target === flyout || flyout.contains(e.target))) {
return;
```
```
The name of the file: chrome/test/data/webui/cr_components/composebox/contextual_action_menu_test.ts
Insertions: 4, Deletions: 0.
@@ -873,11 +873,15 @@
loadTimeData.overrideValues({
contextManagementInComposeboxEnabled: true,
});
+
actionMenu.remove();
actionMenu = document.createElement('cr-composebox-contextual-action-menu');
actionMenu.tabSuggestions = [
createTabSuggestion({tabId: 1, title: 'Tab 1'}),
+ createTabSuggestion({tabId: 2, title: 'Tab 2'}),
+ createTabSuggestion({tabId: 3, title: 'Tab 3'}),
];
+
actionMenu.inputState = new MockInputState({
allowedInputTypes: [InputType.kBrowserTab],
});
```
Change information
Commit message:
[Context-Input] Fix flyout menu positioning on scroll
The share tabs flyout uses `position: fixed` to escape overflow clipping
by its parent `<cr-action-menu>`. Consequently, when the page or parent
dialog scrolls, the flyout remained anchored relative to the viewport
instead of following the Plus menu trigger. This CL attaches a
capture-mode `scroll` listener to `window` whenever the flyout is open
to recalculate the trigger rect and update the flyout coordinates
dynamically.
Before fix:
https://screencast.googleplex.com/cast/NjAwMzEzMjQyMDA2MzIzMnxmMWIxM2VlYS02OQ
After fix:
https://screencast.googleplex.com/cast/NTkyMTY2MjQ2MDQ5MzgyNHxhYzYyMmI3YS1jZg
Fixed: b:524360090
Change-Id: I7c0d913bbf145ebc161d55f10aea1c2255b56690
Reviewed-by: James Leung <james...@google.com>
Commit-Queue: Wenhu Cheng <wen...@google.com>
Cr-Commit-Position: refs/heads/main@{#1652109}
Files:
- M chrome/test/data/webui/cr_components/composebox/contextual_action_menu_test.ts
- M ui/webui/resources/cr_components/composebox/contextual_action_menu.ts
Change size: M
Delta: 2 files changed, 72 insertions(+), 0 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by James Leung
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages