-
Notifications
You must be signed in to change notification settings - Fork 916
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
[Workspace] Limit workspace colors #8607
[Workspace] Limit workspace colors #8607
Conversation
Signed-off-by: Kapian1234 <[email protected]>
Signed-off-by: Kapian1234 <[email protected]>
@@ -129,6 +129,7 @@ export const WorkspaceDetailFormDetails = ({ | |||
error={formErrors.color?.message} | |||
> | |||
<EuiCompressedColorPicker | |||
mode="swatch" | |||
color={formData.color} |
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.
This still allow user to type whatever color they want
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.
Maybe we could add a rule for color validation when submitting like this: euiPaletteColorBlind().includes(color)
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.
should we use same component as create page? for now they are different.
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.
I‘ve tried the validation and it works on both the create page and the detail page
do we need to add validation at api side? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8607 +/- ##
==========================================
- Coverage 60.95% 60.94% -0.01%
==========================================
Files 3790 3790
Lines 90296 90306 +10
Branches 14151 14154 +3
==========================================
- Hits 55036 55034 -2
- Misses 31804 31815 +11
- Partials 3456 3457 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Kapian1234 <[email protected]>
…arch-Dashboards into limit_workspace_colors
@@ -335,11 +335,11 @@ export const validateWorkspaceForm = ( | |||
}), | |||
}; | |||
} | |||
if (color && !validateWorkspaceColor(color)) { | |||
if (color && (!validateWorkspaceColor(color) || !euiPaletteColorBlind().includes(color))) { |
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.
nit: Since we have this !euiPaletteColorBlind().includes(color)
check which inlcudes the logic for !validateWorkspaceColor(color)
, we can remove !validateWorkspaceColor(color)
.
* Limit workspace color Signed-off-by: Kapian1234 <[email protected]> * Limit workspace color at detail page Signed-off-by: Kapian1234 <[email protected]> * Changeset file for PR #8607 created/updated * Add validation Signed-off-by: Kapian1234 <[email protected]> --------- Signed-off-by: Kapian1234 <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 2b3fbf0) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* Limit workspace color * Limit workspace color at detail page * Changeset file for PR #8607 created/updated * Add validation --------- (cherry picked from commit 2b3fbf0) Signed-off-by: Kapian1234 <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
* Limit workspace color Signed-off-by: Kapian1234 <[email protected]> * Limit workspace color at detail page Signed-off-by: Kapian1234 <[email protected]> * Changeset file for PR opensearch-project#8607 created/updated * Add validation Signed-off-by: Kapian1234 <[email protected]> --------- Signed-off-by: Kapian1234 <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
* Limit workspace color Signed-off-by: Kapian1234 <[email protected]> * Limit workspace color at detail page Signed-off-by: Kapian1234 <[email protected]> * Changeset file for PR opensearch-project#8607 created/updated * Add validation Signed-off-by: Kapian1234 <[email protected]> --------- Signed-off-by: Kapian1234 <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
* Limit workspace color Signed-off-by: Kapian1234 <[email protected]> * Limit workspace color at detail page Signed-off-by: Kapian1234 <[email protected]> * Changeset file for PR opensearch-project#8607 created/updated * Add validation Signed-off-by: Kapian1234 <[email protected]> --------- Signed-off-by: Kapian1234 <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
Description
Limit workspace colors to just those in color picker swatch.
Issues Resolved
Screenshot
Testing the changes
Changelog
Check List
yarn test:jest
yarn test:jest_integration