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.

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(core/radio|checkbox): Jumping input controls #1708

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions .changeset/late-hotels-approve.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@siemens/ix": patch
---

__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.
__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.

Fixes #1702
59 changes: 26 additions & 33 deletions packages/core/src/components/checkbox/checkbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
h,
Element,
Method,
Fragment,
} from '@stencil/core';
import { HookValidationLifecycle, IxFormComponent } from '../utils/input';
import { makeRef } from '../utils/make-ref';
Expand Down Expand Up @@ -134,44 +135,36 @@ export class Checkbox implements IxFormComponent<string> {
}

private renderCheckmark() {
if (this.checked) {
return (
<svg
width="18"
height="18"
viewBox="0 0 18 18"
fill="none"
xmlns="http://www.w3.org/2000/svg"
>
return (
<svg
width="18"
height="18"
viewBox="0 0 18 18"
fill="none"
xmlns="http://www.w3.org/2000/svg"
>
{this.indeterminate && (
<Fragment>
<rect width="18" height="18" fill="transparent" />
<rect
x="3"
y="8"
width="12"
height="2"
fill="var(--ix-checkbox-check-color)"
/>
</Fragment>
)}

{this.checked && (
<path
d="M3.65625 8.15625L8.4375 12.9375L14.625 3.9375"
stroke="var(--ix-checkbox-check-color)"
stroke-width="2"
/>
</svg>
);
}

if (this.indeterminate) {
return (
<svg
width="18"
height="18"
viewBox="0 0 18 18"
fill="none"
xmlns="http://www.w3.org/2000/svg"
>
<rect width="18" height="18" fill="transparent" />
<rect
x="3"
y="8"
width="12"
height="2"
fill="var(--ix-checkbox-check-color)"
/>
</svg>
);
}
)}
</svg>
);
}

render() {
Expand Down
32 changes: 32 additions & 0 deletions packages/core/src/components/checkbox/tests/checkbox.ct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,35 @@ test('label', async ({ mount, page }) => {
const checkboxElement = page.locator('ix-checkbox').locator('label');
await expect(checkboxElement).toHaveText('some label');
});

test('Checkbox should not cause layout shift when checked', async ({
mount,
page,
}) => {
await mount(`
<ix-checkbox label="test"></ix-checkbox>
<div id="element-below">This element should not move</div>
`);

await page.waitForSelector('ix-checkbox', { state: 'attached' });

const initialBounds = await page.$eval('#element-below', (el) => {
const rect = el.getBoundingClientRect();
return { top: rect.top, left: rect.left };
});

await page.click('ix-checkbox');

await page.waitForFunction(() => {
const checkbox = document.querySelector('ix-checkbox');
return checkbox?.getAttribute('aria-checked') === 'true';
});

const newBounds = await page.$eval('#element-below', (el) => {
const rect = el.getBoundingClientRect();
return { top: rect.top, left: rect.left };
});

expect(newBounds.top).toBeCloseTo(initialBounds.top, 0);
expect(newBounds.left).toBeCloseTo(initialBounds.left, 0);
});
10 changes: 5 additions & 5 deletions packages/core/src/components/radio-group/test/radio-group.ct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ test('initial checked', async ({ mount, page }) => {
await expect(radioOption2).toHaveClass(/hydrated/);
await expect(radioOption3).toHaveClass(/hydrated/);

await expect(radioOption2.locator('.checkmark')).toBeAttached();
await expect(radioOption2.locator('.checkmark')).toBeVisible();
});

test('change checked', async ({ mount, page }) => {
Expand All @@ -68,14 +68,14 @@ test('change checked', async ({ mount, page }) => {
await expect(radioGroupElement).toHaveClass(/hydrated/);
await expect(radioOption1).toHaveClass(/hydrated/);
await expect(radioOption2).toHaveClass(/hydrated/);
await expect(radioOption2.locator('.checkmark')).toBeAttached();
await expect(radioOption2.locator('.checkmark')).toBeVisible();
await expect(radioOption3).toHaveClass(/hydrated/);

await radioOption3.click();

await expect(radioOption2).not.toHaveAttribute('checked');
await expect(radioOption2.locator('.checkmark')).not.toBeAttached();
await expect(radioOption3.locator('.checkmark')).toBeAttached();
await expect(radioOption2.locator('.checkmark')).not.toBeVisible();
await expect(radioOption3.locator('.checkmark')).toBeVisible();
await expect(radioOption3).toHaveAttribute('checked');
});

Expand Down Expand Up @@ -119,5 +119,5 @@ test('disabled', async ({ mount, page }) => {
);
const radioOption3 = page.locator('ix-radio').nth(2);
await expect(radioOption3).not.toBeEnabled();
await expect(radioOption3.locator('.checkmark')).not.toBeAttached();
await expect(radioOption3.locator('.checkmark')).not.toBeVisible();
});
5 changes: 4 additions & 1 deletion packages/core/src/components/radio/radio.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,10 @@ export class Radio implements IxFormComponent<string> {
}}
onClick={() => this.setCheckedState(!this.checked)}
>
{this.checked && <div class="checkmark"></div>}
<div
class="checkmark"
style={{ visibility: this.checked ? 'visible' : 'hidden' }}
></div>
</button>
<ix-typography
format="label"
Expand Down
32 changes: 32 additions & 0 deletions packages/core/src/components/radio/test/radio.ct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,35 @@ test(`disabled = undefined`, async ({ mount, page }) => {
const disableLabelColor = 'rgba(245, 252, 255, 0.93)';
await expect(label).toHaveCSS('color', disableLabelColor);
});

test('Radio button should not cause layout shift when checked', async ({
mount,
page,
}) => {
await mount(`
<ix-radio label="test"></ix-radio>
<div id="element-below">This element should not move</div>
`);

await page.waitForSelector('ix-radio', { state: 'attached' });

const initialBounds = await page.$eval('#element-below', (el) => {
const rect = el.getBoundingClientRect();
return { top: rect.top, left: rect.left };
});

await page.click('ix-radio');

await page.waitForFunction(() => {
const radio = document.querySelector('ix-radio');
return radio?.getAttribute('aria-checked') === 'true';
});

const newBounds = await page.$eval('#element-below', (el) => {
const rect = el.getBoundingClientRect();
return { top: rect.top, left: rect.left };
});

expect(newBounds.top).toBeCloseTo(initialBounds.top, 0);
expect(newBounds.left).toBeCloseTo(initialBounds.left, 0);
});
Loading