-
Notifications
You must be signed in to change notification settings - Fork 178
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
Adds required to environment variables #3653
base: main
Are you sure you want to change the base?
Adds required to environment variables #3653
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
lgtm, thanks @ashley-o0o! |
<FormGroup isRequired label="Variable type"> | ||
<Split data-testid="environment-variable-field"> | ||
<SplitItem isFilled> | ||
<Stack hasGutter> | ||
<StackItem data-testid="environment-variable-type-select"> | ||
<SimpleSelect | ||
popperProps={{ appendTo: getDashboardMainContainer() }} |
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.
<FormGroup isRequired label="Variable type"> | |
<Split data-testid="environment-variable-field"> | |
<SplitItem isFilled> | |
<Stack hasGutter> | |
<StackItem data-testid="environment-variable-type-select"> | |
<SimpleSelect | |
popperProps={{ appendTo: getDashboardMainContainer() }} | |
<FormGroup isRequired label="Variable type" fieldId="environment-variable-type-select"> | |
<Split data-testid="environment-variable-field"> | |
<SplitItem isFilled> | |
<Stack hasGutter> | |
<StackItem data-testid="environment-variable-type-select"> | |
<SimpleSelect | |
toggleProps={{ id: 'environment-variable-type-select' }} | |
popperProps={{ appendTo: getDashboardMainContainer() }} |
So I noticed this accessibility error when working on my own PR when adding a label to a SimpleSelect. The FormGroup label doesn't point to anything. But you can give the SimpleSelect a toggle props with the id and it will fix it
Closes: RHOAIENG-1201
Description
Previously, when creating a workbench, if a user wanted to add an environment variable, a selection is required to complete the creation. There was no indication that this field was required. This PR adds a required star to the drop down if the user chooses to add the variable.
How Has This Been Tested?
Tested locally
Screenshots
Environment Variables field:
Before:
After:
Test Impact
Request review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
main