Vue3 form components #639
Vue3 form components #639
Conversation
TODO: add custom styling
use a more unique id than just name for fields
- add WIP codebase search component
- added composable API for tagger/codebase (partial)
essentially classes -> composition and getting rid of handlers handlers are replaced with request state that are made available as individual refs in components, which then can decide what to do instead of a handler directly modifying component state behind the scenes
cleaned up where the rule found some style issues but otherwise its not particularly useful for development of vue components nor the requests api
- minor styling fix in form.ts
- add @vueuse/core dep
- should prefer using schema even when validation is not used but resolved the issue with trying to use an undefined schema in useForm anyways Co-authored-by: Allen Lee <alee@users.noreply.github.com>
made date picker polymorphic (Date or string model value) + minor file renaming
- upgrade to bootstrap 5.2
- only use Date model value - add minDate prop to set instead of the client-side validation that is confusing for everyone
Co-authored-by: Allen Lee <alee@users.noreply.github.com>
current thought is to be more explicit within the component about what happens on success/error especially given the design of the api composables giving access to request state
server errors are now packaged up in useAxios and given (along with client-side validation errors from useForm) to a component to display alerts - added more constraints to the client-side date validation approach
which is just a textarea form component with a small indicator icon/link to a markdown guide eventually should be given some editing/previewing functionality once we decide the best way to tackle that (comses/planning#106) [no ci]
includes ux changes such as indicating you need to press enter to add an item and automatically adding an item in the input field on blur (focus lost) as a backup
* missing affiliations field (org search/items component needs to be implemented) - minor refactoring of how props for the vue components are extracted from data attrs/path
- finalized profile edit form (minus a better markdown editor)
|
All form components (minus the placeholder markdown component) are finished, likely along with any major changes to the form api. I'll put in place some component tests to finish off this PR and then work on some documentation for the different apis and then putting together the rest of the pages should be (relatively) straightforward |
There was a problem hiding this comment.
This is a huge pile of excellent work, thanks Scott! I'm good to merge it into the vue3 branch and continue moving forward there in a separate PR / commit stream but would leave it up to you. Left a few minor notes here and there. I think the main thing might be to rename the form components.
There was a problem hiding this comment.
These components are clean and well-designed, great work as usual!
Only nitpicks are in naming:
Should we add a Form prefix to all the components in form/ or just leave them as Alert.vue, Checkbox.vue, etc. The components/form package / dir name hierarchy indicates that they are all form components already. (for more context, see https://devcards.io/smurf-naming-convention and https://softwareengineering.stackexchange.com/questions/191929/the-problems-with-avoiding-smurf-naming-classes-with-namespaces)
Also the CheckboxProps seems like a more generalizable server/validation errors container interface, probably a copypasta that should be AlertProps instead?
There was a problem hiding this comment.
Sort of between a rock and a hard place w/ smurf naming and vue style guide/HTML collisions (https://vuejs.org/style-guide/rules-essential.html#use-multi-word-component-names)
How about getting more descriptive and prepending/appending more specific names?:
Form<...>.vuefor non-field form-level stuff (really only alert for now) -FormAlert.vueField<...>.vuefor field-level 'helpers' like label/error -FieldError.vue<...>Field.vuefor field components -CheckboxField.vue
Otherwise we're looking at more general prefixes, the vue 2 codebases uses C presumably for comses or custom. Here, we'd have to prefix the file name or add another script tag to set the name since <script setup> exports and imports the component as the file name automatically
There was a problem hiding this comment.
Ahh, gotcha - thanks for the style guide link! We could try something like ValidationErrorAlert. So TextArea and TextInput and things would conflict with native <textarea> tags or would it be translated to text-area which might also be confusing 😅
There was a problem hiding this comment.
Yeah since you can use either <text-area> or <TextArea> or any case variation I think it would still cause a conflict there. I tried out my first idea for now: https://github.com/comses/comses.net/tree/db8c099f6a35f0d2b2b9e99a4404b0b5d817949c/frontend-vue3/src/components/form, lmk if you have any thoughts
| } | ||
| for (const key of Object.keys(body)) { | ||
| const value = body[key]; | ||
| if (isISODateString(value)) { |
There was a problem hiding this comment.
could also use Date.parse, try/catch or are we only allowing iso date strings here?
There was a problem hiding this comment.
Yeah the intent was to strictly parse ISO 8601 date strings and assume everything else is meant to be a string, the spec for Date.parse seemed far too loose since most implementations would parse a string like "10 10 10"
- use more descripting prefixes for form components - some type/variable renaming - create src/types.ts for shared types and a `BaseFieldProps` for field components to extend
using imported types with defineProps is currently unsupported but has been fixed awaiting a release in vuejs/core#8083
- minor renaming and refactoring for clarity - make `detailUrl` more robust so it can be used as a general url builder
|
Super helpful review @alee, thanks. Everything here should be addressed. I'll merge this into |
Adds: