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

Improve compatibility for browsers that do not support ResizeObserver or :where selector #5265

Merged
merged 3 commits into from
Jan 30, 2023

Conversation

kylemclean
Copy link
Contributor

@kylemclean kylemclean commented Jan 20, 2023

Description
#5206 made Editable components use stylesheets for default styles instead of inline styles. However, it also inadvertently reduced compatibility with browsers that lack support for ResizeObserver and the :where CSS selector (mainly Safari versions released before September 2020). The result was that browsers that did not support ResizeObserver would throw a ReferenceError when rendering a placeholder element, and browsers that did not support :where would not see default styles for Editable components.

This PR adds a polyfill for ResizeObserver and a fallback for browsers that do not support :where to fix both of those problems.

Issue
Fixes: #5206 (comment)

Context
The latest major browser to add support for ResizeObserver is Safari in version 13.1, released March 2020. (https://caniuse.com/resizeobserver) On browsers that do not support it, a ReferenceError is thrown when a placeholder element is rendered. This problem is fixed by adding a polyfill for ResizeObserver: the npm package @juggle/resize-observer. Note that the native ResizeObserver will be used if it exists.

The latest major browser to add support for :where is Safari in version 14, released September 2020. (https://caniuse.com/mdn-css_selectors_where) Browsers that do not support :where do not parse the default styles for Editable components and placeholder elements, and thus their default styles do not appear.

Default stylesheets using :where were introduced in #5206 to give the default styles for Editable components and placeholder elements a specificity of 0, which allows for user-defined stylesheets to override them by simply adding their own class to the Editable component. I do not know of a way to implement this behavior without support for :where or CSS layers, so I instead opted to just fix the default styles not being applied.

The fallback for the :where selector is implemented by checking support for it using a @supports rule. If there is no support, the default styles for Editable components and placeholder elements is set without :where. This means that after this change, browsers that do not support :where will see default styles, but the specificity of the default styles will be high enough that a user-defined stylesheet with a class for an Editable component will not override the default style. However, inline styles on the Editable component should still override the default styles.

I tested these changes on Firefox 68.9.0 ESR, which supports neither ResizeObserver nor :where, and both problems appear to be fixed.

Note: it is unclear to me which browser versions Slate intends to support, but the FAQ only rules out support for IE11, and in the issue linked above at least one person has expressed a desire for Slate to support pre-13.1 Safari.

Checks

  • The new code matches the existing patterns and styles.
  • The tests pass with yarn test.
  • The linter passes with yarn lint. (Fix errors with yarn fix.)
  • The relevant examples still work. (Run examples with yarn start.)
  • You've added a changeset if changing functionality. (Add one with yarn changeset add.)

@changeset-bot
Copy link

changeset-bot bot commented Jan 20, 2023

🦋 Changeset detected

Latest commit: 6d84877

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
slate-react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@kylemclean kylemclean marked this pull request as ready for review January 22, 2023 09:43
Copy link
Collaborator

@dylans dylans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks useful/reasonable.

We seem to be failing a few integration tests with the change, so that is worth looking into.

@dylans dylans merged commit 3cf51f4 into ianstormtaylor:main Jan 30, 2023
@github-actions github-actions bot mentioned this pull request Jan 30, 2023
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.

2 participants