Expose inputRef and and add required prop to allow html5 validations fixes #93

[zwergius]

[thecodejack]

@zwergius I kind of merged https://github.com/thecodejack/svelte-file-dropzone/pull/105

[zwergius]

@thecodejack ok, but #105 as far as I can see does not expose the inputRef ?

[thecodejack]

oh my bad...can you rebase and add the change?

[Tal500]

@thecodejack ok, but #105 as far as I can see does not expose the inputRef ?

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.

[zwergius]

@thecodejack ok, but #105 as far as I can see does not expose the inputRef ?

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....

[zwergius]

@Tal500 @thecodejack Just rebased and removed formatted changes + added props in README

[Tal500]

One thing that I am now afraid about is that when you export the variable inputElement, you allow it to be modified by the user (Svelte doesn't have readonly props, they are all bidirectional).

Since the value of this var is set after mount and stay fixed until component destruction, I suggest the following: Hide the variable inputElement, but export a function getInputElement (inside the component). This function returns inputElement if the component where already mounted (which happens whenever inputRef != undefined), and throw an exception otherwise. I think it's the safest and clearest solution. No need to worry for varaible reactivity, since the input element stays constant after mounting, before destruction.

[zwergius]

One thing that I am now afraid about is that when you export the variable inputElement, you allow it to be modified by the user (Svelte doesn't have readonly props, they are all bidirectional).

Since the value of this var is set after mount and stay fixed until component destruction, I suggest the following: Hide the variable inputElement, but export a function getInputElement (inside the component). This function returns inputElement if the component where already mounted (which happens whenever inputRef != undefined), and throw an exception otherwise. I think it's the safest and clearest solution. No need to worry for varaible reactivity, since the input element stays constant after mounting, before destruction.

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...

[thecodejack]

I think what @Tal500 referring to is "what will happen if user updates a value the the inputElement?"

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?

[zwergius]

@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...

[Tal500]

I thought about some good compromise. Change the var to be the internal variable _inputElement, do not expose it, and use it(and only it). Then, define the following lines of code:

$: inputElement = _inputElement;
export { inputElement };

This means that the user can bind to the public (reactive) varaible inputElement. If the user decides or by mistake change the variable value, it's his fault and it won't affect the internal logic of the library.

[zwergius]

I thought about some good compromise. Change the var to be the internal variable _inputElement, do not expose it, and use it(and only it). Then, define the following lines of code:

$: inputElement = _inputElement;
export { inputElement };

This means that the user can bind to the public (reactive) varaible inputElement. If the user decides or by mistake change the variable value, it's his fault and it won't affect the internal logic of the library.

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...

[Tal500]

I thought about some good compromise. Change the var to be the internal variable _inputElement, do not expose it, and use it(and only it). Then, define the following lines of code:

$: inputElement = _inputElement;
export { inputElement };

This means that the user can bind to the public (reactive) varaible inputElement. If the user decides or by mistake change the variable value, it's his fault and it won't affect the internal logic of the library.

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.

[zwergius]

@thecodejack Any plans to get this merged at some point ?

[easteryard]

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.

[thecodejack]

I have merged this. Will publish soon