Adding new tokens to Backpack foundation #461
Conversation
# Conflicts: # packages/bpk-foundations-android/tokens/base.raw.android.json # packages/bpk-foundations-common/base/colors/base.json # packages/bpk-foundations-ios/tokens/base.ios.json # packages/bpk-foundations-ios/tokens/base.raw.ios.json # packages/bpk-foundations-web/tokens/base.common.js # packages/bpk-foundations-web/tokens/base.default.scss # packages/bpk-foundations-web/tokens/base.es6.d.ts # packages/bpk-foundations-web/tokens/base.es6.js # packages/bpk-foundations-web/tokens/base.raw.json # packages/bpk-foundations-web/tokens/base.scss
| }, | ||
| "BORDER_RADIUS_FULL": { | ||
| "value": "{!BORDER_RADIUS_FULL}", | ||
| "category": "borders" |
There was a problem hiding this comment.
should this category be radii? same as BORDER_RADIUS_XXX
There was a problem hiding this comment.
Good catch this has been updated 👍
| "value": "{!FONT_SIZE_XS}", | ||
| "type": "font-size", | ||
| "category": "font-sizes", | ||
| "deprecated": true |
There was a problem hiding this comment.
Could I ask why do we add new tokens to packages/bpk-foundations-common/app/legacy-typography.json rather than packages/bpk-foundations-common/app/typography.json. Seems we are adding new tokens with "deprecated": true. Is it intentional?
There was a problem hiding this comment.
Yes actually this is also valid as well. They have been moved to typography.json, legacy will only hold deprecated ones now
|
Can you add a link to the Figma design, and Jira ticket please @EmmanuelBuraimo ? |
| @@ -28,15 +28,23 @@ | |||
| }, | |||
| "BORDER_SIZE_SM": { | |||
| "value": "{!BORDER_SIZE_SM}", | |||
| "category": "borders" | |||
| "category": "radii" | |||
There was a problem hiding this comment.
I think the category of BORDER_SIZE_SM and BORDER_SIZE_LG should be borders while BORDER_RADIUS_FULL and BORDER_RADIUS_NAV_TABS should be radii, which is same as other platform 😄 https://github.com/Skyscanner/backpack-foundations/pull/461/files#diff-c9797f67a52b1ecfbb3f5b6c200e0d05e133fa6c90ea8a14cff56338104c6278
There was a problem hiding this comment.
I have updated this :)
Co-authored-by: GC Zhu <171069086@qq.com>
Co-authored-by: GC Zhu <171069086@qq.com>
…-119-Tabs-New-Token
|
This PR is breaking backpack-ios. There will need to be a fix in iOS to accept this change. Most likely in fonts.js script and backpack itself |
| "FONT_FAMILY_LARKEN": { | ||
| "value": "{!FONT_FAMILY_LARKEN}", | ||
| "type": "font", | ||
| "category": "typesettings" |
There was a problem hiding this comment.
Where have these font changes come from?
|
This can be approved and released as we have to manually update foundations anyway on the app side. This WILL break backpack-ios which we are aware of. I have created to do the required work before we merge it into iOS: https://skyscanner.atlassian.net/browse/DON-1827 |
This pull request introduces updates to border radius tokens across multiple platforms and upgrades a dependency in the
package.jsonfile. The most notable changes include the addition of new border radius tokens (BORDER_RADIUS_FULLandBORDER_RADIUS_NAV_TABS) and a version bump for thelernapackage.Token Updates Across Platforms:
packages/bpk-foundations-android/tokens/base.raw.android.json): AddedBORDER_RADIUS_FULL(value: 9999) andBORDER_RADIUS_NAV_TABS(value: 18).packages/bpk-foundations-ios/tokens/base.raw.ios.json): AddedBORDER_RADIUS_FULL(value: 9999) andBORDER_RADIUS_NAV_TABS(value: 18).packages/bpk-foundations-web/tokens/base.raw.json): AddedBORDER_RADIUS_FULL(value: "100%") andBORDER_RADIUS_NAV_TABS(value: "1.125rem").packages/bpk-foundations-common/app/borders.json): AddedBORDER_RADIUS_FULL(value: 9999) andBORDER_RADIUS_NAV_TABS(value: 18).Dependency Update:
Thanks for contributing to Backpack 🙏
Please include a description of the changes you are introducing and some screenshots if appropriate.
-->
The typography additions will be done once this is confirmed fine