Skip to content

Commit 96c8f78

Browse files
fix(core/radio|checkbox): Jumping input controls
Co-authored-by: Lukas Maurer <lukas.maurer@siemens.com>
1 parent 21ea4b3 commit 96c8f78

File tree

8 files changed

+305
-39
lines changed

8 files changed

+305
-39
lines changed

.changeset/late-hotels-approve.md

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
"@siemens/ix": patch
3+
---
4+
5+
__ix-radio__: Now doesn't change height/layout anymore when clicked. This is achieved by changing the way one of the component's divs is rendered.
6+
__ix-checkbox__: Now doesn't change height/layout anymore when clicked. This is achieved by changing the way one of the component's SVGs is rendered.
7+
8+
Fixes #1702

packages/core/src/components/checkbox/checkbox.tsx

+26-33
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
h,
1919
Element,
2020
Method,
21+
Fragment,
2122
} from '@stencil/core';
2223
import { HookValidationLifecycle, IxFormComponent } from '../utils/input';
2324
import { makeRef } from '../utils/make-ref';
@@ -134,44 +135,36 @@ export class Checkbox implements IxFormComponent<string> {
134135
}
135136

136137
private renderCheckmark() {
137-
if (this.checked) {
138-
return (
139-
<svg
140-
width="18"
141-
height="18"
142-
viewBox="0 0 18 18"
143-
fill="none"
144-
xmlns="http://www.w3.org/2000/svg"
145-
>
138+
return (
139+
<svg
140+
width="18"
141+
height="18"
142+
viewBox="0 0 18 18"
143+
fill="none"
144+
xmlns="http://www.w3.org/2000/svg"
145+
>
146+
{this.indeterminate && (
147+
<Fragment>
148+
<rect width="18" height="18" fill="transparent" />
149+
<rect
150+
x="3"
151+
y="8"
152+
width="12"
153+
height="2"
154+
fill="var(--ix-checkbox-check-color)"
155+
/>
156+
</Fragment>
157+
)}
158+
159+
{this.checked && (
146160
<path
147161
d="M3.65625 8.15625L8.4375 12.9375L14.625 3.9375"
148162
stroke="var(--ix-checkbox-check-color)"
149163
stroke-width="2"
150164
/>
151-
</svg>
152-
);
153-
}
154-
155-
if (this.indeterminate) {
156-
return (
157-
<svg
158-
width="18"
159-
height="18"
160-
viewBox="0 0 18 18"
161-
fill="none"
162-
xmlns="http://www.w3.org/2000/svg"
163-
>
164-
<rect width="18" height="18" fill="transparent" />
165-
<rect
166-
x="3"
167-
y="8"
168-
width="12"
169-
height="2"
170-
fill="var(--ix-checkbox-check-color)"
171-
/>
172-
</svg>
173-
);
174-
}
165+
)}
166+
</svg>
167+
);
175168
}
176169

177170
render() {

packages/core/src/components/checkbox/tests/checkbox.ct.ts

+32
Original file line numberDiff line numberDiff line change
@@ -82,3 +82,35 @@ test('label', async ({ mount, page }) => {
8282
const checkboxElement = page.locator('ix-checkbox').locator('label');
8383
await expect(checkboxElement).toHaveText('some label');
8484
});
85+
86+
test('Checkbox should not cause layout shift when checked', async ({
87+
mount,
88+
page,
89+
}) => {
90+
await mount(`
91+
<ix-checkbox label="test"></ix-checkbox>
92+
<div id="element-below">This element should not move</div>
93+
`);
94+
95+
await page.waitForSelector('ix-checkbox', { state: 'attached' });
96+
97+
const initialBounds = await page.$eval('#element-below', (el) => {
98+
const rect = el.getBoundingClientRect();
99+
return { top: rect.top, left: rect.left };
100+
});
101+
102+
await page.click('ix-checkbox');
103+
104+
await page.waitForFunction(() => {
105+
const checkbox = document.querySelector('ix-checkbox');
106+
return checkbox?.getAttribute('aria-checked') === 'true';
107+
});
108+
109+
const newBounds = await page.$eval('#element-below', (el) => {
110+
const rect = el.getBoundingClientRect();
111+
return { top: rect.top, left: rect.left };
112+
});
113+
114+
expect(newBounds.top).toBeCloseTo(initialBounds.top, 0);
115+
expect(newBounds.left).toBeCloseTo(initialBounds.left, 0);
116+
});

packages/core/src/components/radio-group/test/radio-group.ct.ts

+5-5
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ test('initial checked', async ({ mount, page }) => {
4848
await expect(radioOption2).toHaveClass(/hydrated/);
4949
await expect(radioOption3).toHaveClass(/hydrated/);
5050

51-
await expect(radioOption2.locator('.checkmark')).toBeAttached();
51+
await expect(radioOption2.locator('.checkmark')).toBeVisible();
5252
});
5353

5454
test('change checked', async ({ mount, page }) => {
@@ -68,14 +68,14 @@ test('change checked', async ({ mount, page }) => {
6868
await expect(radioGroupElement).toHaveClass(/hydrated/);
6969
await expect(radioOption1).toHaveClass(/hydrated/);
7070
await expect(radioOption2).toHaveClass(/hydrated/);
71-
await expect(radioOption2.locator('.checkmark')).toBeAttached();
71+
await expect(radioOption2.locator('.checkmark')).toBeVisible();
7272
await expect(radioOption3).toHaveClass(/hydrated/);
7373

7474
await radioOption3.click();
7575

7676
await expect(radioOption2).not.toHaveAttribute('checked');
77-
await expect(radioOption2.locator('.checkmark')).not.toBeAttached();
78-
await expect(radioOption3.locator('.checkmark')).toBeAttached();
77+
await expect(radioOption2.locator('.checkmark')).not.toBeVisible();
78+
await expect(radioOption3.locator('.checkmark')).toBeVisible();
7979
await expect(radioOption3).toHaveAttribute('checked');
8080
});
8181

@@ -119,5 +119,5 @@ test('disabled', async ({ mount, page }) => {
119119
);
120120
const radioOption3 = page.locator('ix-radio').nth(2);
121121
await expect(radioOption3).not.toBeEnabled();
122-
await expect(radioOption3.locator('.checkmark')).not.toBeAttached();
122+
await expect(radioOption3.locator('.checkmark')).not.toBeVisible();
123123
});

packages/core/src/components/radio/radio.tsx

+4-1
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,10 @@ export class Radio implements IxFormComponent<string> {
162162
}}
163163
onClick={() => this.setCheckedState(!this.checked)}
164164
>
165-
{this.checked && <div class="checkmark"></div>}
165+
<div
166+
class="checkmark"
167+
style={{ visibility: this.checked ? 'visible' : 'hidden' }}
168+
></div>
166169
</button>
167170
<ix-typography
168171
format="label"

packages/core/src/components/radio/test/radio.ct.ts

+32
Original file line numberDiff line numberDiff line change
@@ -72,3 +72,35 @@ test(`disabled = undefined`, async ({ mount, page }) => {
7272
const disableLabelColor = 'rgba(245, 252, 255, 0.93)';
7373
await expect(label).toHaveCSS('color', disableLabelColor);
7474
});
75+
76+
test('Radio button should not cause layout shift when checked', async ({
77+
mount,
78+
page,
79+
}) => {
80+
await mount(`
81+
<ix-radio label="test"></ix-radio>
82+
<div id="element-below">This element should not move</div>
83+
`);
84+
85+
await page.waitForSelector('ix-radio', { state: 'attached' });
86+
87+
const initialBounds = await page.$eval('#element-below', (el) => {
88+
const rect = el.getBoundingClientRect();
89+
return { top: rect.top, left: rect.left };
90+
});
91+
92+
await page.click('ix-radio');
93+
94+
await page.waitForFunction(() => {
95+
const radio = document.querySelector('ix-radio');
96+
return radio?.getAttribute('aria-checked') === 'true';
97+
});
98+
99+
const newBounds = await page.$eval('#element-below', (el) => {
100+
const rect = el.getBoundingClientRect();
101+
return { top: rect.top, left: rect.left };
102+
});
103+
104+
expect(newBounds.top).toBeCloseTo(initialBounds.top, 0);
105+
expect(newBounds.left).toBeCloseTo(initialBounds.left, 0);
106+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
/*
2+
* SPDX-FileCopyrightText: 2024 Siemens AG
3+
*
4+
* SPDX-License-Identifier: MIT
5+
*
6+
* This source code is licensed under the MIT license found in the
7+
* LICENSE file in the root directory of this source tree.
8+
*/
9+
import type { ArgTypes, Meta, StoryObj } from '@storybook/web-components';
10+
import type { Components } from '@siemens/ix/components';
11+
import { genericRender, makeArgTypes } from './utils/generic-render';
12+
import { action } from '@storybook/addon-actions';
13+
14+
type Element = Components.IxCheckbox & {
15+
defaultSlot: string;
16+
['checked-change']: any;
17+
validation: string;
18+
'text-on': string;
19+
};
20+
21+
type GroupElement = Components.IxCheckboxGroup & {
22+
label: string;
23+
defaultSlot: string;
24+
};
25+
26+
const CheckboxRender = (args: Element) => {
27+
const container = genericRender('ix-checkbox', args);
28+
const ixCheckbox = container.querySelector(
29+
'ix-checkbox'
30+
) as HTMLIxCheckboxElement;
31+
ixCheckbox.addEventListener('checkedChange', action('checkedChange'));
32+
return container;
33+
};
34+
35+
const CheckboxGroupRender = (args: GroupElement) => {
36+
const container = genericRender('ix-checkbox-group', args);
37+
const checkboxGroup = container.querySelector(
38+
'ix-checkbox-group'
39+
) as HTMLIxCheckboxGroupElement;
40+
checkboxGroup.setAttribute('label', args.label || 'Group');
41+
42+
const checkbox1 = document.createElement('ix-checkbox');
43+
checkbox1.setAttribute('label', 'Checkbox 1');
44+
checkbox1.setAttribute('name', 'checkbox1');
45+
checkbox1.addEventListener('checkedChange', action('checkbox1Change'));
46+
47+
const checkbox2 = document.createElement('ix-checkbox');
48+
checkbox2.setAttribute('label', 'Checkbox 2');
49+
checkbox2.setAttribute('name', 'checkbox2');
50+
checkbox2.addEventListener('checkedChange', action('checkbox2Change'));
51+
52+
checkboxGroup.appendChild(checkbox1);
53+
checkboxGroup.appendChild(checkbox2);
54+
container.appendChild(checkboxGroup);
55+
56+
return container;
57+
};
58+
59+
const meta = {
60+
title: 'Example/Checkbox',
61+
tags: [],
62+
render: (args) => CheckboxRender(args),
63+
argTypes: makeArgTypes<Partial<ArgTypes<Element>>>('ix-checkbox', {
64+
validation: {
65+
control: { type: 'select' },
66+
},
67+
}),
68+
parameters: {
69+
design: {
70+
type: 'figma',
71+
url: 'https://www.figma.com/design/r2nqdNNXXZtPmWuVjIlM1Q/iX-Components?node-id=42365-150769&p=f&t=eGUQESg89t8bPyiB-0',
72+
},
73+
},
74+
} satisfies Meta<Element>;
75+
76+
export default meta;
77+
type Story = StoryObj<Element>;
78+
type GroupStory = StoryObj<GroupElement>;
79+
80+
// More on writing stories with args: https://storybook.js.org/docs/writing-stories/args
81+
export const Default: Story = {
82+
args: {
83+
disabled: false,
84+
label: 'Checkbox',
85+
},
86+
};
87+
88+
export const Disabled: Story = {
89+
args: {
90+
disabled: true,
91+
label: 'Checkbox',
92+
},
93+
};
94+
95+
export const Group: GroupStory = {
96+
render: (args) => CheckboxGroupRender(args),
97+
args: {
98+
label: 'Checkbox Group',
99+
},
100+
};

0 commit comments

Comments
 (0)