Skip to content
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

Feat/forms #26

Merged
merged 17 commits into from
Aug 24, 2023
Merged

Feat/forms #26

merged 17 commits into from
Aug 24, 2023

Conversation

thombruce
Copy link
Owner

@thombruce thombruce commented Aug 22, 2023

closes #25

By submitting this pull request, you agree to follow our Code of Conduct: https://github.com/thombruce/.github/blob/main/CODE_OF_CONDUCT.md


Internal use. Do not delete.

  • Tests passing
  • Coverage sufficient
  • Manual review
  • No A11y regression
  • Translations provided or not needed

@thombruce
Copy link
Owner Author

Add basic support for the following DaisyUI components:

  • Checkbox
  • Radio
  • Toggle

Reconsider component placement

Presently the new form components live at components/content/Form/[name].vue.

I've been exclusively using them within the FormWrapper component and the new Form layout.

If we intend to make them individually usable from within markdown content, this should be documented. If not, they should be moved out of the components/content/ directory.

@thombruce
Copy link
Owner Author

Notes on potential relocation/renaming:

Despite being located in the components/content/Form/ directory, the components are not actually called Form[Name] but just [Name] when used in .vue files. This means components like Input and Select are sharing the namespace with the native HTML elements.

If we change this (and we might), it should still remain possible to use the shorthand we've already been using in the YAML template, meaning the dynamic component loading needs a minor adjustment.

I believe it would be preferable, particularly in markdown files, to access them via some name like :tnt-input rather than :input. Although... provided that they consistently remain a superset of the native element, this actually shouldn't be at all problematic. They're just... native elements enhanced. But it's worthwhile to be aware of this in case of issue.

@thombruce
Copy link
Owner Author

The combobox also isn't presently as customisable as I would like. See the example it's modelled after and aiming to be a replacement for on The Definitive Edition: https://github.com/thombruce/TheDefinitiveEdition/blob/main/components/content/SimpleSearchBar.vue

We could make it possible to pass whatever component or template script a person would want to the <li> elements of the options list.

This may be left as an experiment for another PR.

@thombruce
Copy link
Owner Author

thombruce commented Aug 23, 2023

All form components should also support binding to a Vue object.

Still Todo

  • Allow binding to Vue object/prop/whatever
  • Reconsider directory placement

I'm certain I would like these to be usable from content files, so components/content/ is correct, but we should reconsider nesting them in a Form/ directory since doing so provides little utility... despite being good for organisation, and that alone is worth maintaining the scope for. So maybe just consider whether it is the right name, whether it should be lowercased or modified slightly.

@thombruce
Copy link
Owner Author

Directory changed to forms/ and lowercased. This, I think, slightly alleviates potential confusion about usage.

It is worth considering whether tnt-forms deserves to be a separate library. I think long-term it certainly does - not every site will use the form components, they are potentially just bloat in many, many cases. This raises further questions about separation of, say, @thombruce/tnt-blogs, @thombruce/tnt-docs, @thombruce/tnt-forms should all exist to separate scoped concepts.

For the time being, I am happy to contain all components, layouts and utilities within the one core space. But this is worthwhile to consider for the future.

@thombruce
Copy link
Owner Author

Here's the basic code for binding a component to v-model:

<script>
export default {
  props: ['modelValue'],
  emits: ['update:modelValue']
}
</script>

<template>
  <input
    :value="modelValue"
    @input="$emit('update:modelValue', $event.target.value)"
  />
</template>

The present usage does not pass any values to the new form components - wait, no, that isn't a true; a few do... but most don't - so they're already open to having this adaptation implemented.

@thombruce
Copy link
Owner Author

I'm discovering that usage from within a content markdown file may require a different naming convention.

:input{type="text" name="fields[name]" label="Name" labelAlt="?" hint="This is how your name will be displayed" hintAlt="e.g. Clark Kent" placeholder="What do people call you?"}

...uses the native <input /> element.

:tnt-input{type="text" name="fields[name]" label="Name" labelAlt="?" hint="This is how your name will be displayed" hintAlt="e.g. Clark Kent" placeholder="What do people call you?"}

...uses the TntInput.vue component after I've renamed it.

I guess the idea of enhanced native components is an unworkable one. There are places it would work... For instance, it has been working as a dynamic component, and will work when called with capitalisation from .vue files, but it does not work for .md content files.


Option 1

Rename all components Tnt[Component].vue and prefix dynamic call in FormWrapper.vue with tnt-.

Option 2

Create a secondary group of components, exclusively for content/ usage that hand over all execution to the existing base components.

This would mean moving the existing component set into the components/ root so that the current behaviour still works, and creating a new set of Tnt-prefixed components in components/content/*/ that hand over all handling to those root components.


I'm not sure which way I'm going to go on this. Option 1 has less bloat and more consistency. Option 2 has the nicety of being able to use the components pretty much as if they were native elements (at least outside of content/). Option 2 would also allow me to, to a certain degree, only public expose the props I would like. For instance, Checkbox.vue has been given a klass prop, used to change its class; this is utilised by Toggle.vue but it isn't supposed to be used by the user directly. Despite this... I am leaning towards a Tnt-prefixed approach in all cases. Maybe a later version changes this or some utility is provided to modify it, but I think it's the best approach for the time being.

@thombruce
Copy link
Owner Author

thombruce commented Aug 24, 2023

The v-model binding documented above probably varies for things like radios and checkboxes. Maybe something like this:

<input
  v-model="modelValue"
  :value="option.value || option"
  @input="$emit('update:modelValue', $event.target.value)"
  type="radio"
  :name="name"
  class="radio"
/>

Note the additional use of v-model to pass the binding to the element. I worry this may not work.

  • Create a pages/[some-name].vue testing page with objects and visual outputs to bind for testing form elements

@thombruce thombruce merged commit d91c6a2 into main Aug 24, 2023
@thombruce thombruce deleted the feat/forms branch August 24, 2023 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: Create easy to use form components
1 participant