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

Partial polyfill for CSS 'visibility' #35

Merged
merged 1 commit into from
Feb 26, 2024
Merged

Partial polyfill for CSS 'visibility' #35

merged 1 commit into from
Feb 26, 2024

Conversation

necolas
Copy link
Contributor

@necolas necolas commented Feb 24, 2024

Implements a partial polyfill for non-inheritable 'visibility'. This does not polyfill the web behavior of being able to selectively override the 'visibility' of elements nested within an element with 'visibility:hidden'.

As result, this patch also removes 'visibility' from the inheritable text styles. Originally visibility was added here simply because it is considered an inheritable property on web, even though we didn't do anything with the property on native. In the context of these polyfills, we're only interested in being able to inherit text styles and cannot do anything with inherited visibility, so there is no functional change to the existing polyfill for inheritable text styles.

Also note that no attempt is made to implement 'collapse' as per W3C spec, as this behavior does not appear to be supported for flex items in Chrome anyway (which behave the same whether 'collapse' or 'hidden' is used.)

Copy link

github-actions bot commented Feb 24, 2024

compressed-size: runtime library

Size change: +0.04 kB
Total size: 14.92 kB

Filename: gzip (minify) kB size kB change % change
./packages/react-strict-dom/dist/native.js 12.08 (37.83) +0.04 (+0.08) +0.4% (+0.2%) 🔴
View unchanged
Filename: gzip (minify) kB size kB change % change
./packages/react-strict-dom/dist/dom.js 2.84 (8.81) 0.00 (0.00) 0.0% (0.0%)

Copy link
Contributor

@nmn nmn left a comment

Choose a reason for hiding this comment

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

LGTM.

@necolas necolas force-pushed the polyfill-visibility branch from 7235c18 to 3f75b58 Compare February 24, 2024 03:22
Implements a partial polyfill for non-inheritable 'visibility'. This
does not polyfill the web behavior of being able to selectively override
the 'visibility' of elements nested within an element with
'visibility:hidden'.

As result, this patch also removes 'visibility' from the inheritable
text styles. Originally visibility was added here simply because it is
considered an inheritable property on web, even though we didn't do
anything with the property on native. In the context of these polyfills,
we're only interested in being able to inherit text styles and cannot do
anything with inherited visibility, so there is no functional change to
the existing polyfill for inheritable text styles.

Also note that no attempt is made to implement 'collapse' as per W3C
spec, as this behavior does not appear to be supported for flex items in
Chrome anyway (which behave the same whether 'collapse' or 'hidden' is
used.)
@necolas necolas force-pushed the polyfill-visibility branch from 3f75b58 to c631159 Compare February 26, 2024 18:10
@necolas necolas merged commit 18bcf5f into main Feb 26, 2024
6 checks passed
@necolas necolas deleted the polyfill-visibility branch February 26, 2024 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants