[TASK-915] Simplify button component (even more) #5046
Merged
Conversation
This file contains 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
and update all the instances <3
…gation is more consistent/secure also adds some useful comments
# Conflicts: # jsapp/js/projects/projectsTable/projectQuickActions.tsx # jsapp/scss/libs/react-tagsinput.scss
# Conflicts: # jsapp/js/account/security/email/emailSection.component.tsx # jsapp/scss/libs/react-tagsinput.scss
7 tasks
# Conflicts: # jsapp/js/components/common/button.scss
duvld
reviewed
Sep 3, 2024
| | 'blue' | ||
| | 'red' | ||
| | 'dark-blue'; | ||
| export type ButtonType = 'primary' | 'secondary' | 'danger' | 'secondary-danger' | 'text'; |
There was a problem hiding this comment.
Tiny nitpick. Is there a way to add a jsdoc to this type such that it shows up when auto-completing? At first glance these names are a bit vague and requires the user to read this component whereas before it was a bit more descriptive.
There was a problem hiding this comment.
(Just checking it is the same on vscode also)
There was a problem hiding this comment.
There is a JSDoc comment for the line where type is defined, but I will move it to the line where button prop is - it should do the trick 🆗
duvld
approved these changes
Sep 3, 2024
| | 'blue' | ||
| | 'red' | ||
| | 'dark-blue'; | ||
| export type ButtonType = 'primary' | 'secondary' | 'danger' | 'secondary-danger' | 'text'; |
There was a problem hiding this comment.
(Just checking it is the same on vscode also)
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.
Checklist
Description
Minimizes the amount of button types we use in the UI. There are still some legacy buttons being used (that don't look properly), but the vast majority of buttons will now fall into 5 distinct types. This is an improvement from current 9 types, and from previous 15 types we've had for the last few years.
Improved the accessibility of Button: pending state is no longer focusable and keyboard-clickable, disabled button is no longer TAB focusable
Notes
Things changed here:
type+colormix, but rather unified and simplifiedtype(we have 5 types:primary,secondary,danger,secondary danger, andtext)<Button>to use the new typeRelated issues
Includes work from #5033
After merging this kobotoolbox/docs#372 can be done