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

Feature: mapUnitless on createEmotion, allow Rems as default for unitless values #977

Closed
philholden opened this issue Oct 31, 2018 · 1 comment

Comments

@philholden
Copy link

  • emotion version: any
  • react version: any

I have been converting a UI library to use rems rather than px for font sizes, heights and widths etc to make it accessible for visually impaired users. Most of the work involved finding all unitless values and wrapping them in a rems function. After doing this everything just seemed to work. This made me think it might good to be able to configure this at the library level via a mapUnitless function. Users can then choose to make rems the rule and px the exception. i.e. default to accessible.

const mapUnitless = x => `${x / 16}rem`
createEmotion(context, mapUnitless),

css({
  margin: 16,
  fontSize: 24
})
.css-sd324 {
  margin: 1rem;
  font-size: 1.5rem;
}

https://codesandbox.io/s/py9x54q71q

The above Codesandbox has this feature hacked in (have a play). The cool thing about doing things in this way is that a component designed to work with rems. Will still work if the Emotion configured to use px. There may be a better way to do this what are your thoughts?

@Andarist
Copy link
Member

Thanks for providing a really nice demo - this a great way of showcasing the problem and possible solution!

The whole idea has some problems, e.g. outlined here. At this point in time, I'm not sure how we could accommodate this along with simpler use cases (such as probably yours as you might want to use this only internally~ for your app's purposes). This is a general problem with context-based stuff - not exclusive to your problem at hand (and React is primary integration for emotion - even though there is existing vanilla flavor).

I'm going to close this issue now because it's not immediately actionable and we have sort of PoC PR for something like this (#1299). I would recommend watching this PR if you are interested in this. Hope you understand - trying to slim down opened issues.

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