Skip to content

Conversation

Jarrku
Copy link
Contributor

@Jarrku Jarrku commented Mar 30, 2021

Hello,

We're using headlessui/react in a project that requires IE11 support unfortunately.
The code crashes though when we use the Menu.Items component, as IE expects all 4 arguments to be passed to document.createTreeWalker.

The fourth parameter is deprecated in modern browsers however, so I understand if you decline this PR, though I would ask you to consider this change :)

Similar issue on another project for some more context: webcomponents/webcomponentsjs#556

This was the only instance in the codebase where I could find createTreeWalker usage.

…ional"

Hello,

We're using `headlessui/react` in a project that requires IE11 support unfortunately.
The code crashes though when we use the `Menu.Items` component, as IE expects all 4 arguments to be passed to `document.createTreeWalker`.

The [fourth parameter](https://developer.mozilla.org/en-US/docs/Web/API/TreeWalker/expandEntityReferences) is deprecated in modern browsers however, so I understand if you decline this PR, though I would ask you to consider this change :)

Similar issue on another project for some more context: webcomponents/webcomponentsjs#556

This was the only instance in the codebase where I could find `createTreeWalker` usage.
@vercel
Copy link

vercel bot commented Mar 30, 2021

@Jarrku is attempting to deploy a commit to the Tailwind Labs Team on Vercel.

A member of the Team first needs to authorize it.

RobinMalfait added a commit that referenced this pull request Apr 9, 2021
We got a PR to fix the createTreeWalker so that it also works in IE11.
We don't actively support IE11, so if things work (with polyfills) then
it's good but I don't want to maintain IE11 specific code.

That said, I wanted to abstract the createTreeWalker code to a nice
little hook. The fix for IE is also pretty small, it uses a function
instead of an object and it has a last argument that is deprecated, but
has no obvious effect for our use cases.

Since the incoming PR was based on the `main` branch (where we only had
1 reference to createTreeWalker), I wanted to make sure that we got all
the references on the latest `develop` branch.

Closes: #295
Co-authored-by: Simon VDB <simonvdbroeck@gmail.com>
RobinMalfait added a commit that referenced this pull request Apr 9, 2021
* add useTreeWalker hooks

We got a PR to fix the createTreeWalker so that it also works in IE11.
We don't actively support IE11, so if things work (with polyfills) then
it's good but I don't want to maintain IE11 specific code.

That said, I wanted to abstract the createTreeWalker code to a nice
little hook. The fix for IE is also pretty small, it uses a function
instead of an object and it has a last argument that is deprecated, but
has no obvious effect for our use cases.

Since the incoming PR was based on the `main` branch (where we only had
1 reference to createTreeWalker), I wanted to make sure that we got all
the references on the latest `develop` branch.

Closes: #295
Co-authored-by: Simon VDB <simonvdbroeck@gmail.com>

* use useTreeWalker hook

Co-authored-by: Simon VDB <simonvdbroeck@gmail.com>
@RobinMalfait
Copy link
Member

Hey! Thank you for your PR!
Much appreciated! 🙏

This PR was based on the main branch which is fine, but I created another PR based on the develop branch (#316).
I also mentioned in the commit that we don't actively support IE11, so if it works it's good, but I don't want to do huge refactors. That said, since this change was fairly small and easy to maintain, I added it anyway.

The main reason why I created a new PR is that I wanted to make hooks for them so that the logic is nicely abstracted.

I also added you as a co-author, so that you are still credited for the initial work you did here, and that it still counts as a contribution!

Going to close this PR, since the code is available in the other PR (#316).


This should be fixed, and will be available in the next release.
You can already try it using npm install @headlessui/react@dev or yarn add @headlessui/react@dev.

@Jarrku
Copy link
Contributor Author

Jarrku commented Apr 9, 2021

Yeah that's fine, thank you for adding the change :)

Will base future PRs from the develop branch!

@Jarrku Jarrku deleted the patch-1 branch April 9, 2021 11:07
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

Successfully merging this pull request may close these issues.

2 participants