feat(button): set default shape to soft for ios and round for md and ionic themes #29404
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
I updated the buttons without a shape set in this test to use the soft shape because it doesn't make sense using the default round shape that ionic & md themes use since the round shape is excluded from some of the styles:
:host(.button-full:not(.button-round)) .button-native {
@include border-radius(0);
border-right-width: 0;
border-left-width: 0;
}
| /** | ||
| * Shape="rectangular" is only available in the Ionic theme. | ||
| */ | ||
| // TODO do we need this test for focused / activated? it should be for all themes if so |
There was a problem hiding this comment.
Need to determine if we need this test or if there is a way to slim it down / move it.
There was a problem hiding this comment.
I ended up removing this test because the activated state does not get changed based on the shape & we are checking that the focused state is the proper shape in the clear button tests.
There was a problem hiding this comment.
Question unrelated to this PR: Is test supposed to be capturing the ripple effect? It seems like a normal state button.
There was a problem hiding this comment.
Yeah it looks like it should be:
test.describe(title('button: ripple effect'), () => {
test('should not have visual regressions', async ({ page }) => {
await page.goto(`/src/components/button/test/basic?ionic:_testing=false`, config);
const button = page.locator('#default');
await button.scrollIntoViewIfNeeded();
const boundingBox = await button.boundingBox();
if (boundingBox) {
await page.mouse.move(boundingBox.x + boundingBox.width / 2, boundingBox.y + boundingBox.height / 2);
await page.mouse.down();
}
await page.locator('#default.ion-activated').waitFor();
await expect(button).toHaveScreenshot(screenshot(`button-ripple-effect`));
});
});I am not sure when it stopped working though
...tates/button.e2e.ts-snapshots/button-activated-outline-color-ios-ltr-Mobile-Chrome-linux.png
Outdated
Show resolved
Hide resolved
Issue number: internal
What is the current behavior?
The
shapeproperty defaults to"soft"foriosand"round"for themdandionicthemes.Default button size:
ionic)ios)md)undefined8px14px4px"round"999px"rectangular"0pxLarge button size:
ionic)ios)md)undefined8px16px4pxSmall button size:
ionic)ios)md)undefined4px6px4pxWhat is the new behavior?
The
shapeproperty defaults toundefinedwhich evaluates to the "Soft" shape for all themes.Default button size:
ionic)ios)md)"soft"8px6px4px"round"999px999px999px"rectangular"0px0px0pxLarge button size:
ionic)ios)md)soft8px8px4pxSmall button size:
ionic)ios)md)soft4px4px4px"soft"shape in theionictheme using the existing values forundefined"rectangular"and"round"shapes iniosandmdusing0pxand999pxborder radius"round"for theionicandmdthemes and"soft"foriosand updates theborder-radiusto apply to the shape classes instead of:host"soft"shape border radius foriosin the various sizes to match the buttons created in SwiftUI (their "rounded" is our "soft"):"ionic"theme so that they are not styled based on the size while always following the proper aspect ratio. This was broken for the default size due to an incorrect padding value & I made it so we don't have to specify the padding for each size:Does this introduce a breaking change?
BREAKING CHANGE:
The
border-radiusof theiosandmdbutton now defaults to6pxand999pxinstead of14pxand4px, respectively, in accordance with the iOS and Material Design 3 guidelines. To revert to the previous appearance, set theshapeto"soft"formdand override the--border-radiusCSS variable foriosto14px, or set it to a different value entirely.