-
Notifications
You must be signed in to change notification settings - Fork 62
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
feat(Android): Added tokenized gradients #1059
Conversation
Coverage report
Show new covered files 🐣
Show files with reduced coverage 🔻
Test suite run success471 tests passing in 38 suites. Report generated by 🧪jest coverage report action from a6936b8 |
@@ -260,6 +261,13 @@ const gradients: VkontakteAndroidGradients = { | |||
vkontakteGradientWomensDay: '#FF99CC, #E52E6A', | |||
}; | |||
|
|||
const gradients: Gradients = { | |||
gradient: 'colorBackgroundContent, transparent', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Если оставить так, то при сборке css получится битый токен. Кажется, нужно обернуть их в var(-- )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Теоретически кто-то может заиспользовать эту тему в webview, так что лучше иметь рабочий css
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Типа var(--color_background_content)
?
А есть какое-то более изящное решение, чтобы передать разрабу градиент с именем токена?
У нас же нет чисто строковых токенов?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Мб просто добавить новые переменные, из которых разраб будет собирать градиенты 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Если var(--color_background_content) по каким-то причинам не подходит, то в целом можно новые токены тогда завести
А почему вариант с var не нравится?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@8coon
А почему вариант с var не нравится?
Потому что у нас будут CSS-переменные в JSON-файле для мобильных разработчиков. Не очень кросс-платформенно)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А если тогда добавить пост-процессинг этих значений и добалять var(-- только при сборке css-файлов?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Хочется наоборот в JSON засунуть эти алиасы (не в виде CSS), чтобы у разраба были не только сырые цвета. Если есть идеи, как это запостпроцессить, буду рад.
Если идей нет, то будем локально решать.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Можно дотюнить функцию alias
из файла tokenHelpers.ts
, но она работает так, что подставляет значение в итоговый токен. То есть если например указать в теме:
fontHeadline: alias('fontHeadline1'),
fontHeadline1: 12,
в сборке будет два токена --vkui--font_headline: 12px
и --vkui--font_headline1: 12px
. Я не уверен, что это то, что нужно.
Можешь пожалуйста описать задачу чуть подробнее?
feat/tokenized-gradients: wip new gradient helper function
Упростил описание градиентов в теме Android, чтобы разрабы могли сами их парсить и привязывать к внутренним токенам вместо «плоских цветов».
struct.json до:
struct.json после: