Repository navigation
Conversation
This was referenced Sep 11, 2026
tang-vu
marked this pull request as ready for review
September 11, 2026 16:47
This was referenced Sep 13, 2026
This was referenced Sep 16, 2026
Collaborator
|
Thanks for this contribution. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Hub artifact picker now opens as a native modal, focuses Close, handles Escape through the existing import guard, and restores focus to the repository trigger when closed.
Why this matters
Model Library -> Hugging Face -> Choose file previously left keyboard focus on the background repository list. Its ARIA modal attributes did not contain focus or implement Escape, making artifact inspection difficult with a keyboard.
Overlap check
Searched open/closed PRs by Hugging Face, dialog, keyboard, focus and HuggingFaceModelBrowser.jsx. #2755 labels the close button; #3050 cancels details requests; #3203 covers formatting. #4339 addresses search response races in a separate effect. None establishes this modal lifecycle. #4341 fixes a different dependency-confirmation workflow.
Validation
A rendered-browser-component regression failed on the base because Close did not receive focus; it now passes (1 test). It asserts cancel performs no import and restores focus. Focused ESLint: 0 errors. Native Tab containment is provided by showModal; JSDOM stubs do not validate browser inertness. Native Chromium validation is recorded below.
Tradeoffs and rollback
The existing guard continues to prevent closing during import. Import authorization, API calls and artifact verification are unchanged. Reverting only restores the previous overlay. Uses the modern native dialog API supported by the dashboard's target browsers.
Batch integration receipt
Validated with the other 19 public-beta PRs on integration commit ce0d0132, based on upstream f5f49b7. All 20 patches compose without conflicts in creation order; no new PR requires another to fix its own behavior. For shared files, use #4325 before #4327 and #4339 before #4356 as the tested order.
Chromium on Windows, using the real component and CSS with mocked API data: initial focus, Tab/Shift+Tab, inert background, Escape, and trigger focus restoration all pass. Screenshots were visually inspected. No services were enabled or artifacts imported in this browser check.
Follow-up 2dfa8cc updates the existing Models page test's native-dialog stubs; all 44 page and dialog tests pass. This fixes the CI integration test failure without weakening the production modal API.
Latest candidate CI: 32 successful checks, 4 skipped. Draft status on filesystem/authorization changes is retained for independent human review despite green CI.