Skip to content
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.

Sign up for GitHub

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

fix(pie-icons-webc): DSW-1539 Remove :host-context usage for icon sizing #1107

Merged
merged 16 commits into from
Dec 20, 2023
Merged

fix(pie-icons-webc): DSW-1539 Remove :host-context usage for icon sizing #1107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
ec3e084
fix(pie-icons-webc): DSW-1539 atempt to resolve icon sizing issue
kevinrodrigues Dec 13, 2023
3aa960d
Merge branch 'main' into dsw-1539-icon-size-bug
kevinrodrigues Dec 13, 2023
1d6cb39
fix(pie-icons-webc): DSW-1539 test fix
kevinrodrigues Dec 13, 2023
68de7e8
Merge branch 'main' into dsw-1539-icon-size-bug
kevinrodrigues Dec 14, 2023
296acb5
fix(pie-icons-webc): DSW-1539 rename class
kevinrodrigues Dec 14, 2023
9d33050
Merge branch 'main' into dsw-1539-icon-size-bug
kevinrodrigues Dec 14, 2023
a953e81
Merge branch 'main' into dsw-1539-icon-size-bug
kevinrodrigues Dec 14, 2023
0529a6a
Update .changeset/few-gifts-shop.md
kevinrodrigues Dec 15, 2023
151a397
fix(pie-icons-webc): DSW-1539 removes js and uses css to fix host sty…
kevinrodrigues Dec 15, 2023
491fa64
fix(pie-icon-button): DSW-1539 remove snapshot ref
kevinrodrigues Dec 15, 2023
11a9770
fix(pie-icons-webc): DSW-1539 revert js changes
kevinrodrigues Dec 15, 2023
cb31a05
Merge branch 'main' into dsw-1539-icon-size-bug
kevinrodrigues Dec 18, 2023
86773ae
Merge branch 'main' into dsw-1539-icon-size-bug
kevinrodrigues Dec 18, 2023
babe44e
Update .changeset/few-gifts-shop.md
kevinrodrigues Dec 19, 2023
2869c14
fix(pie-icon-button): DSW-1539 provide fallback for button icon host
kevinrodrigues Dec 19, 2023
3f77cd7
Merge branch 'main' into dsw-1539-icon-size-bug
kevinrodrigues Dec 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/few-gifts-shop.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@justeattakeaway/pie-icons-webc": minor
---

[Fixed] - Remove `:host-context` usage for icon sizing
1 change: 1 addition & 0 deletions packages/components/pie-button/src/button.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ $breakpoint-wide: 768px;
--btn-height--small: 40px;
--btn-height--medium: 48px;
--btn-height--large: 56px;
--btn-icon-display: block;

/**
* Mixin for updating the button styles based on the size passed in.
Expand Down
13 changes: 7 additions & 6 deletions packages/components/pie-icon-button/src/iconButton.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,21 @@
// Normally we don't expect consumers to override these, but there are situations where it may be necessary
:host {
// The base values are set to the size default, which is for the medium button size
--btn-dimension: 48px;
--btn-dimension-default: 48px;

// Sizing is set to Medium button icon size, as that is the default
--btn-icon-size: 24px;
--btn-icon-size-default: 24px;
}

// Base button styles
.o-iconBtn {
--btn-border-radius: var(--dt-radius-rounded-e);
--btn-bg-color: var(--dt-color-interactive-brand);
--btn-icon-fill: var(--dt-color-content-interactive-primary);
--btn-icon-display: block;

block-size: var(--btn-dimension);
inline-size: var(--btn-dimension);
block-size: var(--btn-dimension, var(--btn-dimension-default));
inline-size: var(--btn-dimension, var(--btn-dimension-default));

border-color: var(--btn-border-color);
border-radius: var(--btn-border-radius);
Expand All @@ -36,8 +37,8 @@
}

svg {
height: var(--btn-icon-size);
width: var(--btn-icon-size);
height: var(--btn-icon-size, var(--btn-icon-size-default));
width: var(--btn-icon-size, var(--btn-icon-size-default));
}

&[variant='primary'] {
Expand Down
9 changes: 4 additions & 5 deletions packages/tools/pie-icons-webc/build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,10 @@ const componentSelector = '${kebabCase(name)}';
export class ${name} extends LitElement implements IconProps {
// The following styles make sure that the icon will be sized correctly
static styles = css\`
:host-context(pie-icon-button) svg,
:host-context(pie-button) svg {
display: block;
width: var(--btn-icon-size);
height: var(--btn-icon-size);
:host svg {
Copy link
Contributor

@dandel10n dandel10n Dec 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change will mess up icon size for all our icons making the svg container 24px X 24px. Here is a Link component with an icon as an example:
Screenshot 2023-12-21 at 10 49 26

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a PR for the fix: #1127

display: var(--btn-icon-display, inline);
width: var(--btn-icon-size, 24px);
height: var(--btn-icon-size, 24px);
}
\`;

Expand Down
Loading