Enable use_experimental_non_local_form for all SelectPanels#3713
Conversation
🦋 Changeset detectedLatest commit: 879a515 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull Request Overview
This PR removes the experimental feature flag use_experimental_non_local_form from SelectPanel and enables its functionality for all instances. The change simplifies the SelectPanel API by removing conditional logic and making form arguments consistently supported across all fetch strategies.
- Removes the
use_experimental_non_local_formparameter and related conditional logic - Updates documentation to reflect that form arguments are now supported for all fetch strategies
- Removes the feature flag usage from preview files
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| static/info_arch.json | Removes experimental flag documentation and updates form_arguments description |
| static/arguments.json | Removes experimental flag documentation and updates form_arguments description |
| previews/primer/alpha/select_panel_preview/remote_fetch_form.html.erb | Removes use_experimental_non_local_form flag from preview |
| app/components/primer/alpha/select_panel.rb | Removes experimental flag parameter and conditional logic |
| .changeset/cyan-readers-prove.md | Adds changeset entry for the feature |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @form_builder = form_arguments[:builder] | ||
| @value = form_arguments[:value] | ||
| @input_name = form_arguments[:name] |
There was a problem hiding this comment.
With the removal of the conditional check, these form argument assignments will now execute for all SelectPanel instances. Consider adding validation to ensure form_arguments contains the expected keys when they are being accessed, to prevent potential nil assignment issues.
| @form_builder = form_arguments[:builder] | |
| @value = form_arguments[:value] | |
| @input_name = form_arguments[:name] | |
| form_arguments = form_arguments || {} | |
| @form_builder = form_arguments.key?(:builder) ? form_arguments[:builder] : nil | |
| @value = form_arguments.key?(:value) ? form_arguments[:value] : nil | |
| @input_name = form_arguments.key?(:name) ? form_arguments[:name] : nil |
khiga8
left a comment
There was a problem hiding this comment.
Thank you for working on this!
|
Hi @TylerJDev 👋 ! If you have time do you mind reviewing this PR. We are cleaning up a feature flag that has been turned on in dotcom for several months. |
What are you trying to accomplish?
Clean up the
use_experimental_non_local_formfeature flag in SelectPanel.Screenshots
N/A
Integration
Yes, we will need to remove the
use_experimental_non_local_formprop from SelectPanels.List the issues that this change affects.
Closes # (type the GitHub issue number after #)
Risk Assessment
Medium/Low risk. Almost all SelectPanels in prod have this feature flag on. There are a few panels that have been added in the last 3-5 months where this flag was not used.
Anything you want to highlight for special attention from reviewers?
N/A
Accessibility
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.