Skip to content

Commit

Permalink
fix(icon): issue 637 fixing icon misplacement (#638)
Browse files Browse the repository at this point in the history
* issue 637: fixing icon misplacement

* Update components/icon/src/vwc-icon.scss

Co-authored-by: yinon <yinon@hotmail.com>

* issue 637: icon css fixes

* issue 637: icon css fixes

* fixing lint ignore patterns

* issue 637: CR remarks

* issue #637: CR remarks

* align all instances of vwc-icon with new structure

* update icon-button snapshot

Co-authored-by: yinon <yinon@hotmail.com>
Co-authored-by: joelgraham93 <j.r.g@hotmail.com>
  • Loading branch information
3 people authored Feb 5, 2021
1 parent 74befed commit 2de7e94
Show file tree
Hide file tree
Showing 9 changed files with 81 additions and 24 deletions.
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
**/*.js
!**/test/*.js
!**/stories/**/*.js
**/stories/**/*autogenerated*.js

**/*.tsbuildinfo
**/storybook-static
Expand Down
2 changes: 1 addition & 1 deletion __snapshots__/icon button.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
class="mdc-icon-button"
>
<vwc-icon
class="vwc-icon"
class="vvd-icon"
size="small"
type="bin"
>
Expand Down
10 changes: 6 additions & 4 deletions components/button/src/vwc-button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@

&:not(.mdc-button--dense) {
.vvd-icon {
min-height: 18px;
min-width: 18px;
--icon-size: 20px;
}
}

Expand All @@ -65,6 +64,10 @@

.mdc-button {
height: 32px;

.vvd-icon {
--icon-size: 16px;
}
}
}

Expand All @@ -75,8 +78,7 @@
height: 48px;

.vvd-icon {
min-height: 20px;
min-width: 20px;
--icon-size: 24px;
}

> span.trailing-icon {
Expand Down
14 changes: 6 additions & 8 deletions components/icon-button/src/vwc-icon-button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@
overflow: hidden;
}

.vwc-icon {
--size: 20px;
height: var(--size);
width: var(--size);
.vvd-icon {
--icon-size: 20px;
}
}

Expand Down Expand Up @@ -58,16 +56,16 @@
:host([dense]) {
--mdc-icon-button-size: 32px;

.vwc-icon {
--size: 16px;
.vvd-icon {
--icon-size: 16px;
}
}

:host([enlarged]) {
--mdc-icon-button-size: 48px;

.vwc-icon {
--size: 24px;
.vvd-icon {
--icon-size: 24px;
}
}

Expand Down
2 changes: 1 addition & 1 deletion components/icon-button/src/vwc-icon-button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export class VWCIconButton extends MWCIconButton {

protected renderIcon(): TemplateResult {
return html`<vwc-icon
class="vwc-icon"
class="vvd-icon"
size="small"
type="${this.icon}"
></vwc-icon>`;
Expand Down
31 changes: 25 additions & 6 deletions components/icon/src/vwc-icon.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,25 @@
:host { display: inline-block; fill: currentColor; width: 24px; height: 24px; }
:host > slot { margin: 0; padding: 0; display: block; position: relative; width: 100%; height: 100%; }
:host > slot > svg { width: 100%; height: 100%; position: absolute; top: 0; left: 0; }
:host([size=small]) { width: 16px; height: 16px; }
:host([size=medium]) { width: 24px; height: 24px; }
:host([size=large]) { width: 32px; height: 32px; }
$size-variable-name: --icon-size;

:host([size=small]) {
#{$size-variable-name}: 16px;
}
:host, :host([size=medium]) {
#{$size-variable-name}: 24px;
}
:host([size=large]) {
#{$size-variable-name}: 32px;
}

:host {
display: inline-flex;
fill: currentColor;
width: var(#{$size-variable-name});
height: var(#{$size-variable-name});
inline-size: var(#{$size-variable-name});
block-size: var(#{$size-variable-name});

svg {
flex-grow: 1;
align-items: stretch;
}
}
38 changes: 38 additions & 0 deletions components/icon/test/icon.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import '@vonage/vwc-icon';
import {
waitInterval,
isolatedElementsCreation,
textToDomToParent,
assertDistancePixels,
assertComputedStyle,
} from '../../../test/test-helpers.js';

const LOAD_TIME = 400;
const sleep = (ms) => new Promise((res) => setTimeout(res, ms));
Expand Down Expand Up @@ -43,4 +50,35 @@ describe('vwc-icon', () => {
});
});
});

describe('icon layout', () => {
const addElement = isolatedElementsCreation();
const ICON_SIZES = {
small: '16px',
medium: '24px',
large: '32px',
};

for (const [sizeName, sizeValue] of Object.entries(ICON_SIZES)) {
it(`'${sizeName}' icon should be sized property`, async () => {
const [e] = addElement(
textToDomToParent(`<vwc-icon type="home" size="${sizeName}"></vwc-icon>`)
);
await waitInterval(LOAD_TIME);
const svg = e.shadowRoot.querySelector('svg');
assertComputedStyle(e, { width: sizeValue, height: sizeValue });
assertComputedStyle(svg, { width: sizeValue, height: sizeValue });
});

it(`'${sizeName}' icon should be located property`, async () => {
const [e] = addElement(
textToDomToParent(`<vwc-icon type="home" size="${sizeName}"></vwc-icon>`)
);
await waitInterval(LOAD_TIME);
const svg = e.shadowRoot.querySelector('svg');
assertDistancePixels(e, svg, 'top', 0);
assertDistancePixels(e, svg, 'left', 0);
});
}
});
});
4 changes: 2 additions & 2 deletions components/tab-bar/src/vwc-tab.scss
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
}

.vvd-icon {
--icon-size: 20px;
color: var(--mdc-tab-text-label-color-default);
height: 20px;
width: 20px;
vertical-align: sub;
}

:host([active]) {
Expand Down
3 changes: 1 addition & 2 deletions components/textfield/src/vwc-textfield.scss
Original file line number Diff line number Diff line change
Expand Up @@ -175,11 +175,10 @@
}

vwc-icon {
--icon-size: #{textfield-variables.$icon-size};
align-self: center;
padding: 0;
margin: 0 #{textfield-variables.$horizontal-gutter};
min-block-size: textfield-variables.$icon-size;
min-inline-size: textfield-variables.$icon-size;
}
}

Expand Down

0 comments on commit 2de7e94

Please sign in to comment.