Render text editor contents inside shadow DOM by nathansobo · Pull Request #3943 · atom/atom

nathansobo · 2014-10-23T20:39:39Z

Tasks

Fix git diff styling
Fix specs
Fix loss of ability to insert text after autocompleting when the menu doesn't pop up
Add a feature flag

kevinsawicki · 2014-10-23T20:42:10Z

src/theme-package.coffee

Any reason in particular to change these quotes?

No, I think it just happened across a couple commits because I was interpolating a variable into that string for a while.

Themes aren’t applying correctly and there are issues with mini editors but this basically works. I’m leaving the .editor node in the shadow DOM for theme compatibility and because React still wants to render into a wrapper element.

…instead of via the shadow DOM. We always honor the computed font styles of the host element.

This preserves existing theming behavior

React event handlers don’t work because of the shadow DOM

The focusout event doesn’t seem to work for elements in the shadow DOM. Other people seem to share this experience: https://code.google.com/p/chromium/issues/detail?id=378163#c7

If stylesheet files are named with 2 extensions, the first extension is used as the context argument when the package’s stylesheets are loaded. This allows people to target the text editor by naming their stylesheet `index.atom-text-editor.less`.

This avoids a race condition where stylesheets would be added after all packages were deactivated and leak into the next spec.

Signed-off-by: Max Brunsfeld <maxbrunsfeld@gmail.com>

When the shadow DOM is enabled, this happens organically because the focus is abstracted across the shadow boundary. Without that abstraction boundary, we need to pretend that a blur of the hidden input is actually a blur of the entire editor. Signed-off-by: Max Brunsfeld <maxbrunsfeld@gmail.com>

Signed-off-by: Max Brunsfeld <maxbrunsfeld@gmail.com>

nathansobo · 2014-11-05T00:31:14Z

This is ready to merge.
/cc @atom/core

benogle · 2014-11-05T16:31:15Z

spec/spec-helper.coffee

Yay no more react class

Well, not quite. It's still there but no more reason to call it out in a class specifically.

Oh that's what you said.

benogle · 2014-11-05T16:39:30Z

👍 Doesnt look crazy painful with the feature flag. Was wondering how you did the contextual styles with no shadow root, and the global style element looks pretty elegant. 💥 ⛵

nathansobo · 2014-11-05T16:42:00Z

Thanks for the review. I'm going to merge this now so hold on to your 🎩.

Render text editor contents inside shadow DOM

kevinsawicki reviewed Oct 23, 2014
View reviewed changes

benogle mentioned this pull request Oct 23, 2014

Update text editor to use the Shadow DOM #3677

Closed

nathansobo force-pushed the ns-text-editor-shadow-dom branch 4 times, most recently from c655d8a to 16c5187 Compare October 29, 2014 17:37

benogle force-pushed the ns-text-editor-shadow-dom branch from 16c5187 to 494b767 Compare October 30, 2014 21:49

nathansobo force-pushed the ns-text-editor-shadow-dom branch 2 times, most recently from f3f7ac3 to 12a6095 Compare October 30, 2014 22:04

benogle force-pushed the ns-text-editor-shadow-dom branch from 12a6095 to 3d9eb86 Compare October 30, 2014 22:15

nathansobo force-pushed the ns-text-editor-shadow-dom branch from 11203e6 to 8dbdac7 Compare November 4, 2014 16:59

Nathan Sobo added 19 commits November 4, 2014 16:37

Give atom-text-editor elements a shadow root

22212be

Themes aren’t applying correctly and there are issues with mini editors but this basically works. I’m leaving the .editor node in the shadow DOM for theme compatibility and because React still wants to render into a wrapper element.

Hack: Add editor stylesheets to atom-text-editor shadow root

963c92e

Make atom-text-editor have “display: block”

769c6c5

Style mini editor font sizes on atom-text-editor host element

087387e

…instead of via the shadow DOM. We always honor the computed font styles of the host element.

Support context attribute in <atom-styles>

4e8e5a8

Create WeakMap on element creation to support ‘context’ attribute change

3b6189e

Handle events with native handlers to avoid shadow DOM issues with React

2affff3

Load editor stylesheet in shadow root with style manager context param

c2d0b6d

Use atom.styles to activate stylesheets in packages

1a98cb7

Apply syntax theme stylesheets in text editor shadow DOM via atom.styles

5820669

Only update atom-styles children on context attribute change if attached

2b218d2

Fix sourcePath on text editor stylesheet loading

596987f

Move font styling to host element so font preferences work

65f40d6

Assign StylesElement::context on attachment

22f6268

Initialize atom-styles element in editor shadow dom before measuring

5f4fb23

Apply mini and is-focused class to both editor host element and root

2ab5fa4

This preserves existing theming behavior

Use native event handlers instead of React

b86f687

React event handlers don’t work because of the shadow DOM

Get node once to attach event handlers

5d3602d

Avoid traversing through shadow root on mousewheel events

5be21d6

Nathan Sobo and others added 21 commits November 4, 2014 16:37

Don’t handle text editor focus when it already has focus

cf3f1aa

Replace focusout event handlers on hiddenInput shim with blur handlers

dd17e8f

The focusout event doesn’t seem to work for elements in the shadow DOM. Other people seem to share this experience: https://code.google.com/p/chromium/issues/detail?id=378163#c7

Only cancel focus events if the editor is or contains the related target

6f3c53a

Upgrade package-generator to fix specs with shadow DOM

84d1101

Wait for promise resolution on all calls to activatePackage

dd4e7d6

This avoids a race condition where stylesheets would be added after all packages were deactivated and leak into the next spec.

Toggle quotes back

497b4a4

Null-guard component in blur handler

160bb29

⬆️ autocomplete for shadow DOM fix with auto-selecting 1 option

0488fc2

Fix specs

4537e9b

Simplify focus/blur handling

dd7335c

Signed-off-by: Max Brunsfeld <maxbrunsfeld@gmail.com>

Only create a shadow root if editor.useShadowDOM config setting is true

0e57ede

Signed-off-by: Max Brunsfeld <maxbrunsfeld@gmail.com>

Correctly handle focus when shadow DOM is disabled

9690e44

Signed-off-by: Max Brunsfeld <maxbrunsfeld@gmail.com>

💄

7fe9c14

Fix corner cases related to lifecycle state of EditorComponent on events

100af7d

Signed-off-by: Max Brunsfeld <maxbrunsfeld@gmail.com>

Focus the root TextEditorElement in spec instead of component node

dd1e533

Signed-off-by: Max Brunsfeld <maxbrunsfeld@gmail.com>

Test editor focus/blur handling with shadow DOM enabled/disabled

670a710

Signed-off-by: Max Brunsfeld <maxbrunsfeld@gmail.com>

Enable editor.useShadowDOM in all specs

e1d6d55

Add config schema for editor.useShadowDOM

2b2149b

Signed-off-by: Max Brunsfeld <maxbrunsfeld@gmail.com>

Handle focus on hidden input when shadow DOM is disabled

badf172

Signed-off-by: Max Brunsfeld <maxbrunsfeld@gmail.com>

nathansobo force-pushed the ns-text-editor-shadow-dom branch from 8428210 to badf172 Compare November 4, 2014 23:37

benogle reviewed Nov 5, 2014
View reviewed changes

nathansobo pushed a commit that referenced this pull request Nov 5, 2014

Merge pull request #3943 from atom/ns-text-editor-shadow-dom

2c83c3f

Render text editor contents inside shadow DOM

nathansobo merged commit 2c83c3f into master Nov 5, 2014

nathansobo deleted the ns-text-editor-shadow-dom branch November 5, 2014 16:42

as-cii mentioned this pull request Oct 7, 2016

Remove shadow DOM from atom-text-editor #12903

Merged

26 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Render text editor contents inside shadow DOM#3943

Render text editor contents inside shadow DOM#3943
nathansobo merged 68 commits intomasterfrom
ns-text-editor-shadow-dom

nathansobo commented Oct 23, 2014

Uh oh!

kevinsawicki Oct 23, 2014

Uh oh!

nathansobo Oct 23, 2014

Uh oh!

nathansobo commented Nov 5, 2014

Uh oh!

benogle Nov 5, 2014

Uh oh!

nathansobo Nov 5, 2014

Uh oh!

nathansobo Nov 5, 2014

Uh oh!

benogle commented Nov 5, 2014

Uh oh!

nathansobo commented Nov 5, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

nathansobo commented Oct 23, 2014

Tasks

Uh oh!

kevinsawicki Oct 23, 2014

Choose a reason for hiding this comment

Uh oh!

nathansobo Oct 23, 2014

Choose a reason for hiding this comment

Uh oh!

nathansobo commented Nov 5, 2014

Uh oh!

benogle Nov 5, 2014

Choose a reason for hiding this comment

Uh oh!

nathansobo Nov 5, 2014

Choose a reason for hiding this comment

Uh oh!

nathansobo Nov 5, 2014

Choose a reason for hiding this comment

Uh oh!

benogle commented Nov 5, 2014

Uh oh!

nathansobo commented Nov 5, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants