-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[PM-13895] Autofocus on Vault Search Browser #11888
Conversation
Fixed Issues
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #11888 +/- ##
=======================================
Coverage 33.41% 33.41%
=======================================
Files 2841 2841
Lines 89008 89008
Branches 16979 16979
=======================================
Hits 29744 29744
Misses 56927 56927
Partials 2337 2337 ☔ View full report in Codecov by Sentry. |
@Jingo88 I was also looking into this recently as we'll likely need the same for the new Send page. Your solution works, but I was wondering, if this should actually work out of the box using the autofocus.directive as the underlying search.component within Maybe @willmartian might know more about this or the preferred approach? |
@Jingo88 I'll be curious with what @willmartian thinks as I agree with @djsmith85 that it should work out of the box. After a couple implementation attempts, it appears focus is getting stolen to the body. Possibly because of a navigation, just a hunch I didn't validate that. I added a focus event listener in the focus method on the autofocus directive and it was never invoked. Which is puzzling. private focus() {
if (this.focusableElement) {
console.log(this.focusableElement.getFocusTarget()) // prints the correct input element
this.focusableElement.getFocusTarget().addEventListener('focus', (e) => {
console.log(e) // never gets called
})
this.focusableElement.getFocusTarget().focus();
console.log(document.activeElement) // still the body element
} else {
this.el.nativeElement.focus();
}
} |
Edit: Might have found the culprit. will update soon |
Yes, the |
I appreciate it that but we identified the source of the issue. Thanks to @shane-melton and @nick-livefront for all the help. |
🎟️ Tracking
PM-13895
📔 Objective
add setTimeout focus to browser vault-v2-search
📸 Screen Recording
PM-13895-autofocus-browser.mov
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:
) or similar for great changes:memo:
) or ℹ️ (:information_source:
) for notes or general info:question:
) for questions:thinking:
) or 💭 (:thought_balloon:
) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:
) for suggestions / improvements:x:
) or:warning:
) for more significant problems or concerns needing attention:seedling:
) or ♻️ (:recycle:
) for future improvements or indications of technical debt:pick:
) for minor or nitpick changes