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

setFormValue() is incompleted #17

Open
TechQuery opened this issue Dec 22, 2020 · 16 comments
Open

setFormValue() is incompleted #17

TechQuery opened this issue Dec 22, 2020 · 16 comments
Labels
help wanted Extra attention is needed question Further information is requested

Comments

@TechQuery
Copy link

What I expect

- setFormValue(value: string)
+ setFormValue(value: string | File | FormData, state: string | File | FormData)

Related links

@calebdwilliams
Copy link
Owner

Hey, I'd be happy to take a look at a PR addressing this issue, but I'm not sure of a really good way to do this just yet. I'll do some research. I'll ping @BurnedToast as well. My issue with the comment is that I don't know the best way to determine if you want that element to be a file … maybe I can take a look at the type of data being passed to the first argument of setFormValue and modify the mapped input from there?

Let's chat about this one.

@TechQuery
Copy link
Author

My scenario is similar with https://web.dev/more-capable-form-controls/#setting-a-value .

My Preview File Input component needs to set a File value.

@BurnedToast
Copy link

BurnedToast commented Dec 29, 2020

I also thought about opening a new issue on this one. Thanks @TechQuery for doing so.
The determination is pretty simple i guess. You just need to do an value instanceof File check.

I am currently on vacation, but i could invest some more time into this one next year :)

@TechQuery
Copy link
Author

@BurnedToast @calebdwilliams Happy new year, stay strong, and away from COVID-19~

@calebdwilliams
Copy link
Owner

calebdwilliams commented Dec 29, 2020

Looking at this again, I'm not sure it makes sense to add because there doesn't appear to be a reliable way to support this feature in Safari. One of the goals here should be to create a consistent experience as best we can. Having this work in Firefox and not in Safari (or IE11/older versions of Edge) could lead to some apps not working. I'm tempted to mark this as won't fix due to that. I'll keep seeing if there are work arounds for Safari though.

Edit
Here is a Pen with the above technique if you want to play with it in Safari/Firefox/Chrome. If we can figure out how to get consistent behavior across those three (IE11/legacy Edge is a nice to have), I will definitely get this added ASAP.

@BurnedToast
Copy link

BurnedToast commented Jan 4, 2021

After some research I could not find any solution for Safari, that could be integrated into this polyfill.

But I found a possible workaround for my use case, that should also work for you @TechQuery.
As shown above, there is currently no way to create a new FileList in Safari etc. But you dont need a new mutable list if you just want a 100% copy of an input-file-element from within your shadow dom.

So you could just handle a hidden input-file-element by your own and copy the files property. Here is a code pen showing how to transfer the selected files from one input to another:
https://codepen.io/burnedtoast/pen/YzGaBWY

The creation of the "ghost input" outside the shadow root is not included. Hope this helps

@BurnedToast
Copy link

BurnedToast commented Jan 4, 2021

For my FileUpload-Component, that internally uses an input type="file" element, I ended up using attachInternals() to get support for all the other polyfilled stuff (validation etc).

I just replaced the code for "elementInternals.setFormValue" with the following:

if (ElementInternals.isPolyfilled) {
  // only do the workaround when using the polyfill
  if (!this.outOfShadowRootInput) {
    // convert the polyfill input element to type file and hide it by display=none
    this.outOfShadowRootInput = this.elementRef.nativeElement.nextElementSibling;
    this.outOfShadowRootInput.setAttribute('type', 'file');
    this.outOfShadowRootInput.style.display = 'none';
  }
  // transfer the file list from the internal input element to the polyfill element
  this.outOfShadowRootInput.files = this.getInputElement().files;
} else {
  // use the native way
  this.internals.setFormValue(this.getInputElement().files);
}
  • It is a AngularElements code example
  • this.elementRef.nativeElement is the host element of my webcomponent
  • this.outOfShadowRootInput is the hidden polyfill input element
  • this.internals is the ElementInternals-Object returned by attachInternals
  • this.getInputElement() returns the HtmlInputElement for the file selection from within my webcomponent / shadow dom
  • The else case is untested

@TechQuery
Copy link
Author

@BurnedToast Thanks your example codes~

My File Input component did use <input type="file" hidden /> before, but now I pefer a polyfill way in my code.

I feel sad for Safari users...

@calebdwilliams
Copy link
Owner

Same. My buddy recently put this together to illustrate that web devs now have a new problem on our hands.

I’m going to keep this issue open and hopefully add a caveat to the README about this. I am interested in any solution that hits all modern browsers (bonus points for IE11 compatibility).

@calebdwilliams calebdwilliams added the help wanted Extra attention is needed label Jan 4, 2021
@TechQuery
Copy link
Author

TechQuery commented Jan 4, 2021

@calebdwilliams Thanks your buddy, and I'll show https://issafarithenewie.com/ to my clients~

Add one more reason to this site: Every browser on iOS is Safari indeed !

@calebdwilliams calebdwilliams added the question Further information is requested label May 3, 2021
@calebdwilliams
Copy link
Owner

@TechQuery @BurnedToast this seems to work in the latest version of Safari based on the Pen linked above. Can anyone confirm? If so, is anyone interested in opening up a PR for this?

@calebdwilliams
Copy link
Owner

Following up @TechQuery, @BurnedToast is this still a need?

@BurnedToast
Copy link

I did not had the opportunity to test this, since I do not have any Safari test devices currently.
Hope this will change soon. I will give some Feedback, when I get my hands on any.

@TechQuery
Copy link
Author

I did not had the opportunity to test this, since I do not have any Safari test devices currently. Hope this will change soon. I will give some Feedback, when I get my hands on any.

Me too, I'm a Windows/Linux user.

@jpzwarte
Copy link
Contributor

jpzwarte commented Jan 10, 2024

setFormValue breaks compilation: https://github.com/calebdwilliams/element-internals-polyfill/blob/main/src/element-internals.ts#L168 is missing the File type in the first argument.

CleanShot 2024-01-10 at 13 08 43@2x

@jpzwarte
Copy link
Contributor

@calebdwilliams too busy to create a PR atm, but if you want to use the same typing as Typescript, try this: export type FormValue = Parameters<ElementInternals['setFormValue']>[0];

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants