retail-ui icon indicating copy to clipboard operation
retail-ui copied to clipboard

feat(react-ui): add CSSThemeProvider

Open Eazymov opened this issue 2 years ago • 1 comments

У себя в новом проекте не хотелось использовать css-in-js (я хейтер) и для работы с темами из react-ui сделал себе компонент CSSThemeProvider, который берёт все переменные из какой-либо темы react-ui, и объявляет эти переменные как css custom properties, после чего в css можно использовать, например:

color: var(--text-color-default);
background-color: var(--bg-default);

и так далее. Решил поделиться с react-ui :)

Стоит ли добавлять в общую библиотеку контролов или нет? Что дополнить/изменить?

Спорные моменты:

  1. Стоит ли внутри CSSThemeProvider оборачивать children в ThemeContext.Provider для удобства, или так зона ответственности компонента получается не очень (всё-таки наследование свойств CSS и наследование значений реакт-контекста работает по-разному), и пусть лучше юзер сам явно использует ThemeContext.Provider, если нужно?
  2. Стоит ли сделать возможным передачу в value любого объекта с переменными, а не только результат ThemeFactory.create(...)?

Чек-лист перед запросом ревью

  1. Добавлены тесты на все изменения ⬜ unit-тесты для логики ⬜ скриншоты для верстки и кросс-браузерности ⬜ нерелевантно

  2. Добавлена (обновлена) документация ⬜ styleguidist для пропов и примеров использования компонентов ⬜ jsdoc для утилит и хелперов ⬜ комментарии для неочевидных мест в коде ✅ прочие инструкции (README.md, contributing.md и др.) ⬜ нерелевантно

  3. Изменения корректно типизированы ✅ без использования any (см. PR 2856) ⬜ нерелевантно

  4. Прочее ✅ все тесты и линтеры на CI проходят ✅ в коде нет лишних изменений ✅ заголовок PR кратко и доступно отражает суть изменений (он попадет в changelog)

Eazymov avatar Aug 18 '22 21:08 Eazymov

Привет!

Спасибо за ПР. Но мы не спешим брать на поддержку компоненты, потребность в которых точно не известна. Идеально было бы сначала положить на парковку, прорекламировать и посмотреть на статистику использования.

Но, думаю, что идея хорошая. На счет реализации я скорее за то, чтобы ограничить ответственность компонента только лишь трансляцией переменных из ThemeContext в CSS.

zhzz avatar Aug 30 '22 11:08 zhzz