-
Notifications
You must be signed in to change notification settings - Fork 17.4k
Remove shadow DOM from atom-text-editor
#12903
Conversation
Previously this logic lived in atom-reporter, but it seems more reasonable to throw errors in spec-helper instead, so that the test suite fails in CI as well whenever a deprecated method or stylesheet is used.
Signed-off-by: Nathan Sobo <nathan@github.com>
When, after flashing a decoration, the decorated range moved, Atom was showing an additional flash, even if the previous one had already been consumed. This bug originated in `HighlightsComponent`, where we maintained state about a certain highlight's flash count. The problem with this approach, however, is that highlight objects in the component are very volatile, and we could even have more than one for a single decoration (i.e. when such decoration spans multiple tiles). To fix this, we'll now maintain some additional state in `TextEditorPresenter`, which will set a `needsFlash` attribute on the highlight state objects, thereby preventing `HighlightsComponent` from showing the flash animation more than once when the decorated range changes.
Shadow DOM removed from atom-text-editor in atom/atom#12903
In a syntax theme I am working on, I utilise postcss-prefixer to add all the |
Wasn't it possible to prefix class selectors automagically if the stylesheet's filename ended with ".syntax.less"? Or am I remembering wrong? I recall reading about something like that somewhere... |
@Alhadis I've never heard of that… without PostCSS you mean? |
I think that didn't prefix selectors, but it loaded the stylesheet into |
Background
Back in #3943, we started rendering text editor contents inside a shadow DOM boundary to prevent outer style sheets from altering the editor inner contents. Although this introduced many benefits, it carried several tradeoffs too.
In particular, we started duplicating some of the style sheets (
{context: 'atom-text-editor'}
) inside each shadow root so that package and theme authors could still be able to explicitly target the editor visual elements. An alternative solution to this problem that we suggested (before the shadow DOM spec was finalized) was to use/deep/
and::shadow
pseudo selectors, which unfortunately are now being deprecated.Even more importantly, this architecture was getting in the way of making
atom-text-editor
a reusable component, because it made it even more coupled to Atom and its ecosystem. Other problems involved things like #4590, and a more complex user experience for package and theme authors in general. Ultimately, we started feeling like the extra complexity of the shadow DOM wasn't worth its benefits.Removing shadow DOM from
atom-text-editor
This pull request aims at removing the shadow DOM in a way that prevents elements within
atom-text-editor
that we don't fully control (i.e. the grammar scopes applied as CSS classes to every line's inner<span>
) from being mistakenly styled from the outside. Specifically, every syntactic class name will now be prepended withsyntax--
; styling a JavaScript operator, for example, should now look like the following:Given the wide scope of this change, we are trying to reduce package breakage to a minimum by transforming deprecated selectors automatically and emitting a deprecation in deprecation-cop. Applying these transformations can be quite onerous and therefore we have introduced a layer of caching in 07d56b2 to ensure their performance impact stays low after the first time Atom is launched.
Other notable changes include:
Block decorations rendering has been rewritten from scratch (it previously worked only with the shadow DOM).
When the editor is focused,
document.activeElement
will now return the hidden<input>
insideatom-text-editor
, instead ofatom-text-editor
itself. I could find only one third party package relying on this behavior, and I am going to submit a pull request shortly to usedocument.activeElement.closest('atom-text-editor')
instead.Any Grim or style sheet deprecation will now cause specs to fail, see
ensureNoDeprecatedFunctionCalls
andensureNoDeprecatedStylesheets
.Rendering flashes for decorations has been broken for a long time, but removing the shadow DOM highlighted the cause of the issue, and so they have been fixed in 1091b0e.
Conclusion
Removing the shadow DOM is an important step for the editor's future, as doing so will allow us to extract it, clean it up and optimize it. Considering the invasive nature of this change, we are planning to 🚢 it after rolling the railcars next week, so that everyone can build it from master and extensively test it for an entire release cycle.
Before merging this, we will also need to merge the following core package pull requests, so that we stop using any deprecated API or selector, and make the test suite green again:
scopeSelector.toCss()
to return prefixed syntax selectors first-mate#80)/cc: @atom/core