"Save" and "Cancel" buttons for settings sections #11290
Conversation
…y` instead of `isDisabled`)
somebody1234
added
CI: No changelog needed
somebody1234
requested review from
PabloBuchu,
indiv0,
MrFlashAccount,
Frizi,
farmaazon,
vitvakatu,
kazcw and
AdRiley
as code owners
| @@ -26,7 +26,7 @@ interface SubmitButtonBaseProps { | |||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | |||
| readonly form?: FormInstance<any> | |||
| /** Defaults to `submit`. */ | |||
| readonly action?: 'cancel' | 'submit' | 'update' | |||
| readonly action?: 'cancel' | 'save' | 'submit' | 'update' | |||
There was a problem hiding this comment.
how's the save is different from 'submit' or 'update'?
On the second thought, why'd we need that at all? we had formnovalidate which was quirky, but web-compliant, now we have custom prop, that looks very unclear and has very confusing values.
| autoFocus = false, | ||
| ...inputProps | ||
| } = props | ||
| const formFromContext = Form.useFormContext() |
There was a problem hiding this comment.
nit: we can pass the form prop in the context and the context will return the passed form if it's defined, otherwise will try to consume a form from the context.
| @@ -11,6 +11,11 @@ import { tv } from '#/utilities/tailwindVariants' | |||
|
|
|||
| const SETTINGS_INPUT_STYLES = tv({ | |||
| extend: INPUT_STYLES, | |||
| variants: { | |||
| readOnly: { | |||
| true: { base: 'opacity-100', textArea: 'border-transparent' }, | |||
There was a problem hiding this comment.
I think the inputs in settings should be somehow hightlighted, now it's unclear that we can click on the name and edit it, since nothing is making it different from an ordinary text element
somebody1234
added
the
CI: Keep up to date
somebody1234
removed
the
CI: Keep up to date
somebody1234
added
the
CI: Ready to merge
|
@somebody1234 After I click [UPDATE] ok looks like it works but in the background. the new value will appear in the input after reload. please fix |
somebody1234
removed
the
CI: Ready to merge
|
@PabloBuchu yup, good catch. looks like it is changed internally but because it's separate from the form, the displayed form value doesn't change |
|
@PabloBuchu should be fixed now |
somebody1234
force-pushed
the
wip/sb/save-settings-sections
branch
from
f32c10a
to
d15076e
Compare
somebody1234
added
CI: Ready to merge
somebody1234
removed
the
CI: Keep up to date
jdunkerley
added
the
CI: Clean build required
Pull Request Description
Important Notes
None
Screenshots
Before:
After:
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
The documentation has been updated, if necessary.Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
Unit tests have been written where possible.