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

Allow standard html attributes to be passed to input #59

Closed
pechitook opened this issue Jun 5, 2023 · 18 comments
Closed

Allow standard html attributes to be passed to input #59

pechitook opened this issue Jun 5, 2023 · 18 comments

Comments

@pechitook
Copy link

pechitook commented Jun 5, 2023

Hello. I don't have a ton of experience in Svelte yet so maybe this is not an issue, but it would be great to allow any valid html attribute to be applied to the inner input this library exposes. Some examples I can think of

  • id
  • aria- attributes
  • data- attributes

Is there an easy way to do this in Svelte or one would need to create wrapper proprs for all of them? This doesn't scale well with dynamic ones like data- which are custom (valid) attributes. Here's an example of how Vue handles this

Happy to put up a PR once we agree on the strategy here

@pechitook
Copy link
Author

After some proper look at the Svelte documentation, I found https://svelte.dev/docs#template-syntax-attributes-and-props

$$restProps contains only the props which are not declared with export. It can be used to pass down other unknown attributes to an element in a component. It shares the same optimisation problems as $$props, and is likewise not recommended.

<input {...$$restProps}>

I think this could work.

@probablykasper
Copy link
Owner

I'm not too sure about this, what's your use case? I wouldn't want people to for example add a name because that would be a weird format to use and it would only work with the input

@pechitook
Copy link
Author

@probablykasper the most straightforward use case I can think of is adding an id to be able to semantically use a label

<label for="some input>
<input id="some input" />
</label>

@probablykasper
Copy link
Owner

We could add an id prop

@elton2048
Copy link

It is better to allow extra class attributes to be in <input> element too for layout customization using like tailwind css.

@probablykasper
Copy link
Owner

class is supported

@elton2048
Copy link

class is supported

I am not sure if this is the case, but the source doesn't show the class for the <input> element getting the class attributes (I am the <input> element inside the <div> wrapper element).

<input
    class:invalid={!valid}
    type="text"
    value={text}

@probablykasper
Copy link
Owner

probablykasper commented Jun 30, 2023

Ah yeah you're right, it styles the <div> container instead of the <input>. Maybe it would work fine to change it to apply to <input>

@spy16
Copy link

spy16 commented Jul 4, 2023

example add a name because that would be a weird format to use

Could you elaborate on this please? To make it work with forms, this is needed.

Some reasons to allow control on attributes of input:

  • To make it work with forms (e.g., for use with SvelteKit actions), having the ability to set name is useful.
  • To add labels id would be needed.
  • To be able to set all the aria- attributes

Overall, I think allowing all standard input attributes, except where it would interfere with the custom logic of the component would be good idea i think. This way, it would be an enhanced version of input type="date" and allow all the patterns that we can use with it (#UseThePlatform 🙂)

@probablykasper
Copy link
Owner

Could you elaborate on this please? To make it work with forms, this is needed.

The input is a customizable user-facing value, not a data value. Parsing on the server would get complicated, especially for user-dependent formats. It also wouldn't work for the DatePicker component.

I'd be happy to have other attributes that make sense, like id.

Would be good to have built-in aria- attributes

@jhoddevik
Copy link

So ... how can we use this in forms?

@probablykasper
Copy link
Owner

By binding to the date value and putting it in an input element

@eberhapa
Copy link

eberhapa commented Aug 3, 2023

We could add an id prop

That's exactly what I need because of Accessibility reasons to make possible.

@pechitook
Copy link
Author

pechitook commented Aug 3, 2023

It seems we have some alignment that at least id and aria- attributes are needed to have some basic accessibility support.

I think the remaining big item is about the implementation strategy. The way I see it we have at least two potential solutions

1- Expose specific, hardcoded attributes for id, and all the aria- attributes
2- Use something like I suggested initially <input {...$$restProps}> which would allow us to future-proof this library and even support valid custom attributes like data- ones. I could put up a PR just to discuss the approach here by looking at an example

Any pros/cons you all see here? cc @probablykasper

@eberhapa
Copy link

eberhapa commented Aug 3, 2023

What's the problem of giving the possibility to pass all props? It would be the most flexible way.

@probablykasper
Copy link
Owner

I don't want to allow all props for the reasons I already mentioned. id and aria- attributes make sense

@happysalada
Copy link
Contributor

Could we also add a "required" attribute to be passed.
if used in a form and the user didn't input, but is trying to submit, most browser have auto focuse on the required field. This would be nice to have.

@probablykasper
Copy link
Owner

required and id are now supported. For name, see

About aria-, feel free to open a new issue with more concrete details

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

No branches or pull requests

7 participants