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

Shadow DOM support and tests #2337

Closed
wants to merge 2 commits into from
Closed

Shadow DOM support and tests #2337

wants to merge 2 commits into from

Conversation

web-padawan
Copy link

Rebased the work from #1805 on top of the 1.3.6
Submitting this PR to check if the tests pass

@web-padawan
Copy link
Author

Tests are failing in Safari because not implemented getSelection() API.
Potential fix would be using https://github.com/GoogleChromeLabs/shadow-selection-polyfill

@web-padawan
Copy link
Author

Shadow selection polyfill does not work properly because of this line: https://github.com/quilljs/quill/blob/96e38e92637b75b907579d0cc1b201920aebe38c/core/selection.js#L32

See the listener in the polyfill where setTimeout is also used and the timeout is 0: https://github.com/GoogleChromeLabs/shadow-selection-polyfill/blob/9eff233c765a685bbf488b2f8eba0d3d75f44cbd/shadow.js#L74

@kr05
Copy link

kr05 commented Mar 2, 2019

What's the progress on this issue? I tried using quill on a polymer project a couple of months ago and unfortunately I was not able to get it to work. :(

Is there anything I can do to help? I really look forward to using quill in my projects!

EDIT: I just noticed #1805, I'll take a look and hopefully take a shot at helping out.

@alisaono
Copy link

alisaono commented May 5, 2019

Is this support for Shadow DOM still actively worked on? I see #1805 has also been open for a while.

@web-padawan
Copy link
Author

Note, the polyfill for Safari added in this PR mostly works but is not quite stable (as we have observed various issues with Quill which it introduces).

Furthermore, the selection API has to be changed so that there will be getComposedRange() method in future (at least there seems to be a consensus): w3c/selection-api#98

@DeadWisdom
Copy link

@web-padawan -- I need to get this working yesterday. And I'd like to spend some time on it.

  • What issues do you see in Safari?
  • What are the state of the tests in here failing?
  • Do you have any sense of the roadmap for getting this PR to work?

Thanks

@web-padawan
Copy link
Author

@DeadWisdom the problem is described here webcomponents/polyfills#66 (comment)

In our project, we ended up using a fork with custom build, source is here:
https://github.com/web-padawan/quill/commits/vaadin-quill

That branch is derived from 1.3.6 Quill version, so porting it to master might be untrivial.
Also, it contains some tweaks which we only need for our project.

The commits below are especially targeting the issue in Safari, so you should be able to apply these patches on top of 1.3.6 branch yourself, in the following order:

vaadin/quill@2186896

vaadin/quill@31253d6

vaadin/quill@5331cb2

vaadin/quill@56a960e

vaadin/quill@95b8374

Note, the polyfill used by our project is not official, neither 100% tested. We consider it "good enough" to make Quill work in Shadow DOM in Safari but some edge cases will occur.

@DeadWisdom
Copy link

Thank you very much @web-padawan for your work on this. I've been given a small sandbox of time to get this done. So we'll see what I can do.

@frankcliu
Copy link

@DeadWisdom updates?

@robrez
Copy link

robrez commented Oct 7, 2020

Ping? Anything we can do to get this merged?

@gfodor
Copy link

gfodor commented Aug 15, 2022

I ended up working on this problem elsewhere, one thing missing from this PR is properly handling the document.activeElement reference in the bubble theme.

@evanjmg
Copy link

evanjmg commented Dec 6, 2022

It's almost 2023? any updates on support or a forked latest npm package etc?

@luin luin deleted the branch slab:1.3.6 January 21, 2024 14:22
@luin luin closed this Jan 21, 2024
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.

10 participants