Repository navigation
feat: Placeholder DOM elements - #3205
matthewlipski wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| // positions are hardcoded for now | ||
| if (state.doc.content.size === 6) { | ||
| decs.push( | ||
| Decoration.node(2, 4, { |
There was a problem hiding this comment.
I think (had a quick look), you should be able to keep the old code, but change Decoration.node into Decoration.widget (in the right position)?
Your performance regression is likely that the new version loops the document (doc.descendents). From a quick look, I don't understand why all the code needed to change and not just the decoration type
Summary
Currently, DOM elements are rendered as CSS pseudo elements. This is a problem for a11y accessibility testing as we cannot use an axe-core scan to check its contrast. This is because axe only checks contrast for DOM elements. Therefore, this PR changes the implementation to use real DOM elements for placeholders.
This comes with a performance overhead. A script has been added in this PR which benchmarks the render time between the old & new implementations, using a document of 1000 empty paragraphs and 1000 empty list items. It measures the initial render times, as well as re-renders on selection and content changes. The table below shows the performance overhead tested locally.
The alternative solution to this problem is to have additional contrast check tests just for placeholders. To be honest, both that solution and the one in this PR don't seem the cleanest, so I would appreciate feedback r.e. this.
This PR is currently more of an experimental change as I'm unsure whether we want this.
Rationale
TODO
Changes
TODO
Impact
TODO
Testing
TODO
Screenshots/Video
TODO
Checklist
Additional Notes
TODO