search

opentable / design-tokens

A place where OpenTable engineers and designers openly work together
https://opentable.github.io/design-tokens/
MIT License
80 stars 74 forks source link

Update px to rem #523

Closed dethstrobe closed 2 years ago

dethstrobe commented 3 years ago

lgtm, but will moving to a relative unit of measurement cause issues for existing components when merged?

Not unless people are overriding the base html font-size. But we do want to do this, for the purpose of creating a responsive UI and allow for a more accessible site.

ot-raypeters commented 3 years ago

Not unless people are overriding the base html font-size. But we do want to do this, for the purpose of creating a responsive UI and allow for a more accessible site.

I agree on the benefits for responsiveness and accessibility.

Do we know of any apps overriding the base html font size?

dethstrobe commented 3 years ago

css overriding html font-size

And more...

How relevant are some of these repos? Why do we have so much legacy code not archived? Are some of these even maintained?

ronniechong commented 3 years ago

I am for the rem direction and I am also wary the need to retain the pixels tokens if some engineers want to keep using it. Should we have a fallback token if they do not wish to use rem? e.g token.yml has rem, token-pixel.yml has pixels as legacy. Can't think of a better label, -fallback, -legacy, -pixel.

That means, the otkit-spacing npm package will then:

import { spacingXSmall } from 'otkit-spacing/token'; // imports 0.25rem
import { spacingXSmall } from 'otkit-spacing/token-pixel'; // imports 4px

Regardless which direction, we will also need to update our styleguide to reflect the changes e.g. https://opentable.github.io/design-tokens/otkit/typography/

dethstrobe commented 3 years ago

I think we should actually not leave the legacy pixel measurement. I think we should go all in or else there might be some temptation to not move forward with new standards. While there might be some weird edge case breaks, I think it'd still be worth the risk and to find those edge cases and fix them would be more important then supporting legacy standards.

ronniechong commented 3 years ago

I think we should actually not leave the legacy pixel measurement. I think we should go all in or else there might be some temptation to not move forward with new standards. While there might be some weird edge case breaks, I think it'd still be worth the risk and to find those edge cases and fix them would be more important then supporting legacy standards.

I think Consumer side uses these packages heavily so we need to check with their side if deprecating pixels completely the way to go

andreiduca commented 3 years ago

I'm in favor of moving to rem values instead of pixel, BUT... what is the greater goal here? and how will we make sure changing this now will be 100% backwards compatible? what happens with code (components, apps) that upgrades to these new tokens, while still using px values for non-otkit sizes?

i would like a bit more clarity into this upgrade and a concrete proposal and roadmap or migration/upgrade plan.

dethstrobe commented 3 years ago

Goal

Make opentable.com more responsive.

By moving to rems and ems we will be able to scale up or down the content quickly based off of screen size or user preference.

This will also allow us another benefit of making our site more accessible.

Risk

Low. As long as the base font-size is 16px, this will have no effect. For any handful of app that use design tokens and change the base font, the content will scale appropriately, which should have a small effect on layout.

Testing