-
Notifications
You must be signed in to change notification settings - Fork 125
[SelectPanel] single-select form support for eventually_local
#3354
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
Conversation
|
|
60744d5
to
7af8d4f
Compare
b63a6e4
to
20eafe7
Compare
eventually_local
e31155a
to
855cd54
Compare
2e38680
to
f0c7059
Compare
eventually_local
eventually_local
eventually_local
eventually_local
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.
Awesome! LGTM! ✨
Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days. |
Relates to: https://github.com/github/primer/issues/3705#top
What are you trying to accomplish?
This PR introduces test coverage to validate that single-select form,
**eventually_local
** fetch strategy on SelectPanel is supported!I discovered that multi-select for non-local strategies is not yet supported and will additional changes because e JavaScript in
select-panel-element.ts
overrides whatever hidden fields we add server-side. That should be addressed in a separate PR. I've started #3362 to add relevant previews/tests.Integration
None of the eventually_local instances on dotcom involve forms, so this should not have any impact.
Risk Assessment
Anything you want to highlight for special attention from reviewers?
Merge checklist
Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.