refactor: use the new button component on forms and dialogs #16050
Conversation
| @@ -43,10 +43,12 @@ module.exports = { | |||
| secondary: "hsl(var(--surface-invert-secondary))", | |||
| }, | |||
| error: "hsl(var(--surface-error))", | |||
| destructive: "hsl(var(--surface-destructive))", | |||
There was a problem hiding this comment.
I believe that surface-destructive and border-destructive is meant to replace surface-error and border-error
There was a problem hiding this comment.
Good catch! 👍
|
@BrunoQuaresma there are still a few places that still use the existing MUI button. for example in FullPageHorizontalForm.tsx. If there is a specific reason to wait on these, it may be useful to create a separate issue to handle those buttons. |
| {jobError ? "Retry" : "Create template"} | ||
| </Button> | ||
| {logs && ( | ||
| <button |
There was a problem hiding this comment.
Is there a specific reason not to use the new button component here?
| <FormFooter> | ||
| <Button type="submit" disabled={form.isSubmitting}> | ||
| {form.isSubmitting && <Spinner />} | ||
| Create organization |
There was a problem hiding this comment.
I am changing this button in a separate PR being reviewed now. It may be best to not make this change here to avoid merge conflicts.
|
|
||
| <Button type="submit" disabled={isLoading}> | ||
| {isLoading && <Spinner />} | ||
| Create group |
There was a problem hiding this comment.
In the new designs I have seen from Christin, I have seen buttons of this type being named "Save". I could be useful to confirm if the new naming pattern for form buttons is "Cancel" and "Save"
site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPage.test.tsx
Outdated
Show resolved
Hide resolved
⬆️ Let's do that. "submit" indicates that something needs to be reviewed before it becomes active while "save" indicates immediate confirmation and with that comes peace of mind. |
I don’t think we currently have any form actions that require an additional confirmation step, so all the forms would use "Save." That said, I think "Save" is a bit too generic—we could make the action clearer to the user by labeling the button based on the context. For example, if a user is editing a workspace, we might want to label the button as "Save Workspace" instead of just "Save." That being said, I don’t have a strong opinion or any supporting studies/articles about this approach, so I’m fine with sticking to "Save" or "Submit" if that’s preferred. I just wanted to bring this up for discussion! |
|
Let's stick to "Save" as the default. The context should already be very clear in most cases. Using specific naming might not work for all cases or the button could get very wide which might break the page layout. Quick benchmark: |
This is a significant PR that will impact many parts of the UI, so I’d like to ask you, @jaaydenh, for a very thorough review of the Storybook stories on Chromatic. I know it’s a bit of a hassle with around 180 stories affected, but it’s all for a good cause 💪
Main changes:
Buttoncomponent to match the new buttons design.Buttoncomponent.Related to #14978