Initial proposal to support usage in shadow roots #2264

spielzeugland · 2021-06-21T14:54:35Z

This change is a rough and initial proposal allowing to use this
library at least within the same parent shadow root #1659 (but not cross
shadowRoots yet). It does neither cover all functionalities yet nor
is in a quality state to be merged (missing tests etc).

This requires the following aspects

Make sure not using query selectors on document directly as those
cannot find elements inside shadow roots. Therefore use specific DOM
nodes to start searching other nodes
Events are handled centrally, but event.target will reference to the
hosting element. Therefore use event.composedPath() to get the real
nested element for that event.
Shadow roots isolate stylesheets, means global styles do not bleed
into the shadow root. Therefore we need to make sure to generate the
styles into the shadow root. As I couldn't find a way to get the
mounted DOM node for the DragDropContext component at the point in
time where the stylesheets are currently getting generated into the
DOM. Therefore this changes allows consumers to specify the DOM
location as a prop of the DragDropContext. Same approach is taken by
JSS (see https://cssinjs.org/jss-api?v=v10.6.0#setup-jss-instance).

This change is a rough and initial proposal allowing to use this library at least within the same parent shadow root (but not cross shadowRoots yet). It does neither cover all functionalities yet nor is in a quality state to be merged (missing tests etc). This requires the following aspects - Make sure not using query selectors on document directly as those cannot find elements inside shadow roots. Therefore use specific DOM nodes to start searching other nodes - Events are handled centrally, but event.target will reference to the hosting element. Therefore use event.composedPath() to get the real nested element for that event. - Shadow roots isolate stylesheets, means global styles do not bleed into the shadow root. Therefore we need to make sure to generate the styles into the shadow root. As I couldn't find a way to get the mounted DOM node for the DragDropContext component at the point in time where the stylesheets are currently getting generated into the DOM. Therefore this changes allows consumers to specify the DOM location as a prop of the DragDropContext. Same approach is taken by JSS (see https://cssinjs.org/jss-api?v=v10.6.0#setup-jss-instance).

atlassian-cla-bot · 2021-06-21T14:54:37Z

Thank you for your submission! Like many open source projects, we ask that you sign our CLA (Contributor License Agreement) before we can accept your contribution.
If your email is listed below, please ensure that you sign the CLA with the same email address.

The following users still need to sign our CLA:
❌spielzeugland

_{Already signed the CLA? To re-check, try refreshing the page.}

spielzeugland · 2021-06-28T08:30:16Z

Thanks @mrbaguvix,
I consider your approval as an agreement on the approach itself. I guess there are a lot of other scenarios/places which I haven't found yet as I am not a heavy user of the library myself. Would be great if someone could help me finding those cases e.g. by testing scenarios within shadowRoots. In the meantime there are still failing tests I need to fix.

0ex-d · 2021-06-28T20:56:57Z

You can create an issue to this effect maybe the community can better reproduce this.

kehengz

Thanks a lot for the change! It works now for my use case inside shadowRoot, with a minor change mentioned below :)

kehengz · 2021-07-13T03:27:15Z

src/view/use-sensor-marshal/use-sensor-marshal.js

@@ -171,7 +171,7 @@ function tryStart({
    return null;
  }

-  const entry: DraggableEntry = registry.draggable.getById(draggableId);
+  const entry: DraggableEntry = registry.draggable.getById(draggableId, sourceEvent.srcElement.shadowRoot || document);
  const el: ?HTMLElement = findDraggable(contextId, entry.descriptor.id);


You added a param "documentFragment" to findDraggable but the caller does not pass in the argument. Change it to findDraggable(contextId, entry.descriptor.id, sourceEvent.srcElement.shadowRoot || document) works for me

Thanks @kehengz, uploaded an additional commit which also fixes this part. The first commit was more of a hacky sketch.
Out of curiosity, did you tested with React 17 or lower?

Got it thanks! My repo uses React 17

- Clean-up former commit - Adapted more places to work with shadowRoots - Introduced helper functions for querying elements - Support for nested shadowRoots - Added sample stories

gorakong · 2021-07-16T21:11:01Z

hey @spielzeugland, we've received your signed CLA. It's just mapped to a different github username (BobPfeiffer?). @danieldelcore (or other maintainers) feel free to merge this while we get @spielzeugland's entry in our db updated to the correct github account 🙂.

spielzeugland · 2021-07-16T21:33:16Z

Thanks @gorakong and @mrbaguvix for your support - I should have marked this PR as a draft. There is still one TODO left in query-elements.js which I want to address. Probably I should also add some tests before merging?

spielzeugland · 2021-07-20T15:28:40Z

@mrbaguvix I think I am done with the change now. Would be great if you could review it again. As said I worked on allowing shadow roots inside draggables/droppables, added some tests and samples.

Nuurek · 2021-07-24T14:49:34Z

@spielzeugland I've played a bit with your changes - great work, thank you very much for it!

One small fix, in src/view/use-style-marshal/get-styles.js . There are styles that are supposed to be applied globally using body selector. Unfortunately, it won't work in the shadow DOM. It can be fixed by broadening the selector: selector: 'body, :host'.

This solves the problem of missing drop events in sample stories.

spielzeugland · 2021-07-26T07:30:12Z

Thanks @Nuurek, for the trying out and suggesting a fix. I uploaded the changes.

spielzeugland · 2021-08-02T19:11:19Z

Hi @danieldelcore, Hi @alexreardon,
would you mind having a look at the changes? From reading your blogs I know bigger changes are difficult to review and accept. Would be happy to get some feedback.

This fixes missing grabbing cursor while dragging.

spielzeugland · 2021-10-12T07:41:13Z

Moved to a new PR #2319 with rewritten history in order to satisfy the CLA build.

spielzeugland mentioned this pull request Jun 21, 2021

Support usage in shadow roots #1659

Open

spielzeugland changed the title ~~Initial proposal to support usage in shadow roots #1659~~ Initial proposal to support usage in shadow roots Jun 21, 2021

0ex-d approved these changes Jun 24, 2021

View reviewed changes

kehengz suggested changes Jul 13, 2021

View reviewed changes

spielzeugland added 4 commits July 13, 2021 09:50

Continued work

8003260

- Clean-up former commit - Adapted more places to work with shadowRoots - Introduced helper functions for querying elements - Support for nested shadowRoots - Added sample stories

Merge remote-tracking branch 'origin'

fca21fa

Update yarn.lock

02303c0

Fix formatting and size check

1a5a623

spielzeugland force-pushed the proposal1659 branch 2 times, most recently from 422e522 to acdea5b Compare July 13, 2021 19:09

Fix more code style issues

f301a6b

spielzeugland force-pushed the proposal1659 branch 4 times, most recently from b337f7d to 3b787f3 Compare July 15, 2021 07:30

Fix flow validation errors

71fec43

spielzeugland force-pushed the proposal1659 branch from 3b787f3 to 71fec43 Compare July 16, 2021 07:30

0ex-d approved these changes Jul 16, 2021

View reviewed changes

spielzeugland added 5 commits July 19, 2021 16:24

Support for shadowRoots inside draggables/droppables

a5eb9ba

Add tests for stylesInsertionPoint

4a98b8c

Add test for query-elements.js

4c6deff

Pin version of react-shadow-dom-retarget-events

c0addb5

Fix validation error

4b27f95

Increase react-shadow-dom-retarget-events version

ece5160

This solves the problem of missing drop events in sample stories.

Apply global styles to shadow root's host

f7d361a

This fixes missing grabbing cursor while dragging.

spielzeugland force-pushed the proposal1659 branch from 3a69274 to f7d361a Compare August 4, 2021 19:20

spielzeugland closed this Oct 12, 2021

spielzeugland deleted the proposal1659 branch March 7, 2023 19:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial proposal to support usage in shadow roots #2264

Initial proposal to support usage in shadow roots #2264

spielzeugland commented Jun 21, 2021 •

edited

Loading

atlassian-cla-bot bot commented Jun 21, 2021 •

edited

Loading

spielzeugland commented Jun 28, 2021

0ex-d commented Jun 28, 2021

kehengz left a comment

kehengz Jul 13, 2021

spielzeugland Jul 13, 2021

kehengz Jul 13, 2021

gorakong commented Jul 16, 2021 •

edited

Loading

spielzeugland commented Jul 16, 2021

spielzeugland commented Jul 20, 2021

Nuurek commented Jul 24, 2021

spielzeugland commented Jul 26, 2021 •

edited

Loading

spielzeugland commented Aug 2, 2021

spielzeugland commented Oct 12, 2021

Initial proposal to support usage in shadow roots #2264

Initial proposal to support usage in shadow roots #2264

Conversation

spielzeugland commented Jun 21, 2021 • edited Loading

atlassian-cla-bot bot commented Jun 21, 2021 • edited Loading

spielzeugland commented Jun 28, 2021

0ex-d commented Jun 28, 2021

kehengz left a comment

Choose a reason for hiding this comment

kehengz Jul 13, 2021

Choose a reason for hiding this comment

spielzeugland Jul 13, 2021

Choose a reason for hiding this comment

kehengz Jul 13, 2021

Choose a reason for hiding this comment

gorakong commented Jul 16, 2021 • edited Loading

spielzeugland commented Jul 16, 2021

spielzeugland commented Jul 20, 2021

Nuurek commented Jul 24, 2021

spielzeugland commented Jul 26, 2021 • edited Loading

spielzeugland commented Aug 2, 2021

spielzeugland commented Oct 12, 2021

spielzeugland commented Jun 21, 2021 •

edited

Loading

atlassian-cla-bot bot commented Jun 21, 2021 •

edited

Loading

gorakong commented Jul 16, 2021 •

edited

Loading

spielzeugland commented Jul 26, 2021 •

edited

Loading