Skip to content

Commit

Permalink
fix: ignore empty text nodes while setting icon-position attribute in…
Browse files Browse the repository at this point in the history
… button (#390)

Co-authored-by: Christian Pajung <christian.pajung@telekom.de>
Co-authored-by: Arturo Castillo Delgado <ac@iconstorm.com>
  • Loading branch information
3 people authored Jun 8, 2021
1 parent 9c39667 commit d88c2b8
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,25 @@ exports[`Button should handle click 2`] = `
</scale-button>
`;

exports[`Button should handle more than 1 node 1`] = `
exports[`Button should handle icon after 1`] = `
<scale-button icon-position="after" size="small">
<mock:shadow-root>
<button class="button button--icon-after button--size-small button--variant-primary" part="base variant-primary after">
<slot></slot>
</button>
</mock:shadow-root>
Label
<scale-icon-action-search size="16"></scale-icon-action-search>
</scale-button>
`;

exports[`Button should handle icon before 1`] = `
<scale-button icon-position="before" size="small">
<mock:shadow-root>
<button class="button button--icon-before button--size-small button--variant-primary" part="base variant-primary before">
<slot></slot>
</button>
</mock:shadow-root>
<scale-icon-action-search size="16">
Label
</scale-icon-action-search>
Expand Down
19 changes: 17 additions & 2 deletions packages/components/src/components/button/button.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,27 @@ describe('Button', () => {
expect(element.getCssClassMap()).toContain('button--disabled');
});

it('should handle more than 1 node', async () => {
it('should handle icon before', async () => {
const page = await newSpecPage({
components: [Button],
html: `
<scale-button size="small">
<scale-icon-action-search size="16" /> Label
<scale-icon-action-search size="16" />
Label
</scale-button>
`,
});
expect(page.rootInstance.iconPosition).toBe('before');
expect(page.root).toMatchSnapshot();
});

it('should handle icon after', async () => {
const page = await newSpecPage({
components: [Button],
html: `
<scale-button size="small">
Label
<scale-icon-action-search size="16" />
</scale-button>
`,
});
Expand Down
5 changes: 4 additions & 1 deletion packages/components/src/components/button/button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,10 @@ export class Button {
* If so, it's probably an icon, so we set `iconPosition` to `after`.
*/
setIconPositionProp() {
const nodes = Array.from(this.hostElement.childNodes);
const nodes = Array.from(this.hostElement.childNodes).filter((node) => {
// ignore empty text nodes, which are probably due to formatting
return !(node.nodeType === 3 && node.nodeValue.trim() === '');
});
if (nodes.length < 2) {
return;
}
Expand Down

0 comments on commit d88c2b8

Please sign in to comment.