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

The Polyfill doesn't work inside the shadow DOM #30

Closed
russcarver opened this issue Mar 18, 2021 · 11 comments
Closed

The Polyfill doesn't work inside the shadow DOM #30

russcarver opened this issue Mar 18, 2021 · 11 comments

Comments

@russcarver
Copy link

I was using this in my Angular project and we've been converting all of our components to use the shadow DOM since we dropped support for IE11. This polyfill doesn't support that. I tried all of the documented ways to combine label & input, but they all assume that the label is accessible via "document.get..." of some type. With the shadow DOM you need "document.getElementById().shadowRoot.getElementById()". The "shadowRoot" here is the key. And if your component is nested inside another component that also uses the shadow DOM, then you have to use "shadowRoot" yet another time.

It would be far easier to simply pass in the label element - like you allow with the input element - via the 'timePolyFill' function. I brought your code directly into my project and modified it to do just that in order to work with Safari.

Here are the relevant bits (code is in TypeScript):

<label id="myLabel"></label>
<input #time type="time" id="timeInput" formControlName="contentTime">
               
               
import * as timePolyFill from 'lib/time-input-polyfill';

public constructor(private elemRef: ElementRef) {

private someMethod(): void {
  const timeLabelElem: HTMLElement = this.elemRef.nativeElement.shadowRoot.getElementById('myLabel');
  const timeInputElem: HTMLElement = this.elemRef.nativeElement.shadowRoot.getElementById('timeInput');
  timePolyFill(timeInputElem, timeLabelElem);
}

And from the polyfill code:
index.js (just the relevant parts):

function TimePolyfill($input, $labelElem) {

...

  var label;
  if ($labelElem) {
    label = $labelElem.textContent;
  } else {
    label = get_label($input)
  }

...

}
@Dan503
Copy link
Owner

Dan503 commented Mar 18, 2021

Proving the ability to pass in a label as a second parameter sounds like a good solution 🙂👍

Would you like to make a PR? It sounds like you have already done the fix in your own project.

@russcarver
Copy link
Author

russcarver commented Mar 22, 2021

@Dan503 Sure thing. Would I branch off the develop branch and create a PR back to develop? The develop branch looks a bit different from master...

@Dan503
Copy link
Owner

Dan503 commented Mar 22, 2021

Yeah branch off develop branch

@Dan503
Copy link
Owner

Dan503 commented Mar 22, 2021

Oh actually no, branch off master.

I'm not sure what state develop branch is in at the moment.

@Dan503
Copy link
Owner

Dan503 commented Mar 22, 2021

Sorry, I changed my mind again.

Branch off develop. It looks like the changes on develop are just build processes and formatting related.

Trying to merge your changes into develop would suck so it's better if the changes are done directly to develop and not master.

@russcarver
Copy link
Author

@Dan503 I am unable to create a branch in your codebase (I must not have permission). Can you add me as a contributor? Or create a branch for me (feature/shadow-dom)?

@Dan503
Copy link
Owner

Dan503 commented Mar 23, 2021

You need to fork my repository then create a branch off your fork to apply the changes.

You then create a pull request to merge your branch on your forked repository to my repository.

This is the standard way to contribute to open source projects.

@Dan503
Copy link
Owner

Dan503 commented Mar 23, 2021

Also, in the example code you wrote, you did this:

  var label;
  if ($labelElem) {
    label = $labelElem.textContent;
  } else {
    label = get_label($input)
  }

This would be the preferred way to write that:

const label = $labelElem ? $labelElem.textContent : get_label($input)

In develop branch I think I babelify so const should be safe to use.

@russcarver
Copy link
Author

@Dan503 PR is up!

@Dan503
Copy link
Owner

Dan503 commented Mar 26, 2021

Fixed in PR #31

I need to do a bit of testing before I can publish it.

@Dan503
Copy link
Owner

Dan503 commented Apr 13, 2021

Fixed in release 1.0.10

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

2 participants