-
Notifications
You must be signed in to change notification settings - Fork 676
ui: Autofocus search box when navigating to settings, plugins or flags page #4603
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🎨 Perfetto UI Build✅ UI build is ready: https://storage.googleapis.com/perfetto-ci-artifacts/gh-21443884687-1-ui/ui/index.html |
camillobruni
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
| { | ||
| onVisibilityChanged: (visible: boolean, dom: Element) => { | ||
| if (visible) { | ||
| const input = findRef( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: would this be worth a common helper on GateDetector or so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I follow you here..?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean something like GateDetector.focusRef(...) so we don't have to repeat the boilerplate code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack re the boilerplate code. But focusing text inputs by ref is unrelated to GateDetector.
I could get behind a separate utility like focusInputByRef(...) perhaps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM, definitely just nice-to have :)
GateDetectorcomponent that detects when an ancestorGateopens/closesBackground: The Gate Component
The
Gatecomponent is used throughout the UI to wrap pages, tabs, and other content that needs to persist its DOM state when hidden. It works by:display: noneto visually hide content while keeping DOM intactGate is used in:
PageManager- wraps each page so switching pages preserves stateTabswidget - wraps tab content for the same reasonDrawerPanel- wraps bottom panel tabsCurrentSelectionTab- wraps selection detail tabsThe Problem
Components inside a Gate can't tell when they become visible vs hidden. The standard Mithril lifecycle hooks (
oncreate/onremove) only fire once when the DOM is actually created/destroyed, not when the Gate opens/closes. This makes it impossible to do things like "focus the search box when the user navigates to this page".The Solution: GateDetector
GateDetectoris a wrapper component that:[data-gate-open]attributeonVisibilityChanged(visible, dom)when visibility changes (e.g. when the gate opens/closesUsage:
Test plan