-
Notifications
You must be signed in to change notification settings - Fork 48
Expose inputRef and and add required prop to allow html5 validations fixes #93 #102
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
Conversation
@thecodejack ok, but #105 as far as I can see does not expose the inputRef ? |
oh my bad...can you rebase and add the change? |
Indeed, #105 doesn't solves this. Notice that I have a giant PR #107 , so if you want to rebase/merge you better be waiting to the conversation there in my opinion. |
@Tal500 This is actually a very simple PR - I hadn't got around to clean it up yet, most of the changes are prettier which probably should not be there... So the only real addition here is 2 new props: inputRef & required, which are both added to the input. You change being so big, I'd probably like to get this in before. It should be very straightforward to merge.... |
Additionally, fix a potential issue with timout being invoked after destroying. fixes thecodejack#47
@Tal500 @thecodejack Just rebased and removed formatted changes + added props in README |
Removes prepare script Co-authored-by: Tal500 <tal_hd@hotmail.com>
Co-authored-by: Tal500 <tal_hd@hotmail.com>
Co-authored-by: Tal500 <tal_hd@hotmail.com>
One thing that I am now afraid about is that when you export the variable Since the value of this var is set after mount and stay fixed until component destruction, I suggest the following: |
I really don't see that as an issue, this is an optional prop that you set if you need to interact with the inputElement, just as you would with any other element, if you know you need the element, then I reckon you also know not to modify it . Exporting a function isn't very svelte like... |
I think what @Tal500 referring to is "what will happen if user updates a value the the So @Tal500 I am guessing we are binding inputElement with component rather setting value to inputElement. I am guessing it won't be a problem but worth checking before merge. If possible @zwergius can you quick confirm that won't be an issue? |
@thecodejack You mean like setting the file from outside the component ? You'd have to create another input and through file uploaded there set this input.. Not sure I see that as something you need to worry about... |
I thought about some good compromise. $: inputElement = _inputElement;
export { inputElement }; This means that the user can bind to the public (reactive) varaible |
I don't understand why this is such a huge issue, but if you insist on this maybe it's simpler if you just push this change yourself? I only added this in order to tap into some input events that aren't exposed so I can use the native form validation... I could have achieved the same by adding callbacks for the missing events but in the end I thought that this component would be more flexible if exposing the inputElement... |
Just wanted to see the maintainer thoughts before implementing. |
@thecodejack Any plans to get this merged at some point ? |
Do you have an update on this @thecodejack? Got a project which depends on it and would really like to know if we can expect it to be merged any time soon. |
I have merged this. Will publish soon |
No description provided.