LOOM-1274 BpkBreadcrumb Class 2 Overrides #3320
Conversation
jronald01
changed the title
|
Visit https://backpack.github.io/storybook-prs/3320 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3320 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3320 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3320 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3320 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3320 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3320 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3320 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3320 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3320 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3320 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3320 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3320 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3320 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3320 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3320 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3320 to see this build running in a browser. |
| /* After adding the wrapper around the icon, need to set the line | ||
| height to stop the 16x16 or 32x32(zoomed) icon from expanding | ||
| verically past it's 16 or 32 pixels height */ | ||
| $icon-height: tokens.$bpk-line-height-xs / 2; |
There was a problem hiding this comment.
Given this is only used in one place does this need a whole dedicated variable just for one line further down?
There was a problem hiding this comment.
Ah, this is because of Styelint, i.e.:
It's fine if I am just doing:
&__arrow {
margin: 0 tokens.bpk-spacing-sm();
line-height: tokens.$bpk-line-height-xs; <<--------------
fill: tokens.$bpk-text-disabled-day;
}
but when I try and divide it by 2, stylelint wants a separate variable.
There was a problem hiding this comment.
The other option is I can set it to a specific pixel value of no greater than 12 pixels before the alignment starts to break again for the regular and zoomed in icon, e.g.:
line-height: 12px;
Or the other option is:
line-height: 0; which forces the element to only use the vertical space it needs.
But both will need a stylelint disable.
Or line-height: 75%, line-height: 0.75, etc.
But again, all will need stylelint disable:
42:5 ✖ Expected variable, function or keyword for "0.75" of "line-height" scale-unlimited/declaration-strict-value
There was a problem hiding this comment.
Ok gotcha! What happens if you wrap it with brackets line-height: (tokens.$bpk-line-height-xs / 2)?
Does it still throw the warning then? If yes then ok to go with this approach
There was a problem hiding this comment.
Ah yeah, I actually first attached a screenshot with brackets and then I replaced it for one without the brackets:
There was a problem hiding this comment.
So with or without brackets produces the same result basically.
There was a problem hiding this comment.
No worries in which case approved!
|
Visit https://backpack.github.io/storybook-prs/3320 to see this build running in a browser. |
* BpkBreadcrumb LOOM-1274 Class 2 Overrides * Fix Styellint issues and update snapshots * missed tokens prefix * fix alignment * fix alignment 02 * fix alignment 03 * fix alignment 04 * fix icon height * fix alignment 05 * fix alignment 06 * fix alignment 07 * fix alignment 08 * fix alignment 08 * fix alignment 09 * revert back to bpktext * set specific icon height for regular and zoom * set specific value that ensures percy snapshots still match * header comment was moved slightly --------- Co-authored-by: James Ronald <james.ronald@skyscanner.net>
* BpkBreadcrumb LOOM-1274 Class 2 Overrides * Fix Styellint issues and update snapshots * missed tokens prefix * fix alignment * fix alignment 02 * fix alignment 03 * fix alignment 04 * fix icon height * fix alignment 05 * fix alignment 06 * fix alignment 07 * fix alignment 08 * fix alignment 08 * fix alignment 09 * revert back to bpktext * set specific icon height for regular and zoom * set specific value that ensures percy snapshots still match * header comment was moved slightly --------- Co-authored-by: James Ronald <james.ronald@skyscanner.net>
Add existing Bpk child component overrides to wrappers.
Had to set a specific line height after adding the wrapper around the 16x16 or 32x32(zoomed) icon to retain the correct alignment, i.e. adding the wrapper increases the height of the right arrow icon which then affects the alignment of the right arrow within the breadcrumb.
Without the line height fix, the right arrows are vertically shifted up:
With the line height fix, the right arrows are back to where they should be centrally aligned:
[KOA-123][BpkButton] Updating the colourREADME.md(If you have created a new component)README.md.d.ts) files updated