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

Inconsistent output of r if no units are specified #119

Closed
smhigley opened this issue May 24, 2022 · 4 comments
Closed

Inconsistent output of r if no units are specified #119

smhigley opened this issue May 24, 2022 · 4 comments

Comments

@smhigley
Copy link

If the r SVG CSS prop is specified with no units, griffel will generate a className + style for Chromium browsers, but will create a className with no style for Firefox. You can see the difference in this codesandbox on each browser:
https://codesandbox.io/s/musing-roentgen-r9i7g5?file=/src/App.js

Firefox has these classNames/styles:

.f1uzb8hk { width: 45px; }
.fvfw249 { height: 45px; }
.f4a1i41 { cx: 50%; }
.f15az5lx { cy: 50%; }
.f10o6nd5 { }

Chromium has these:

.f1uzb8hk { width: 45px; }
.fvfw249 { height: 45px; }
.f4a1i41 { cx: 50%; }
.f15az5lx { cy: 50%; }
.f10o6nd5 { r: 10; }

It seems like it would be more predictable and much easier to debug if griffel were to output the r style regardless of browser support for units/unitless values.

@layershifter
Copy link
Member

TL;DR It's not a problem in Griffel.


I forked CodeSandbox and have done the same with plain CSS, https://codesandbox.io/s/brave-resonance-fcx04n?file=/src/App.js.

Chrome

image

Firefox

image


According to MDN:

For properties defined in CSS2, a length unit identifier must be provided. For length values in SVG-specific properties and their corresponding presentation attributes, the length unit identifier is optional.

https://developer.mozilla.org/en-US/docs/Web/SVG/Content_type#length

This looks like a bug in Firefox. Feel free to report it 😉

@smhigley
Copy link
Author

@layershifter there's also the separate issue with Firefox, but the problem with Griffel is that the r value is not compiled into CSS at all in Firefox, which makes the issue quite difficult to debug. It'd be ideal if the experience with an element using styles compiled by griffel were exactly the same as in your codesandbox.

@layershifter
Copy link
Member

layershifter commented May 26, 2022

@smhigley Griffel generates the same CSS in all browsers and tries to insert it. The problem comes from .insertRule() behavior - invalid rules (from browser's POV) are not inserted, there is nothing that we can do to solve it (using CSS extraction #53 potentially improves it as experience will be the same as with CSS).

Minimal example: https://codesandbox.io/s/kind-rosalind-teiu3m?file=/src/index.js

Chrome

image

Firefox

image

@smhigley
Copy link
Author

@layershifter that makes sense, thanks! Yeah, the CSS extraction would be nice, and in the meantime logging the output to the console works ¯_(ツ)_/¯

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

No branches or pull requests

2 participants