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

feat(button): add circular shape as round #29161

Merged
merged 31 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
b02881f
feat(button): add circular shape as round
thetaPC Mar 13, 2024
59621b9
chore(): add updated snapshots
Ionitron Mar 13, 2024
233a056
chore(): add updated snapshots
Ionitron Mar 14, 2024
76d7f45
refactor(button): update styles
thetaPC Mar 15, 2024
bea547b
Merge branch 'FW-6017' of github.com:ionic-team/ionic-framework into …
thetaPC Mar 15, 2024
8be455c
refactor(button): add comments
thetaPC Mar 15, 2024
43269c5
chore(): add updated snapshots
Ionitron Mar 15, 2024
2874898
refactor(button): add comment for isCircle
thetaPC Mar 15, 2024
6fd9fdd
Merge branch 'FW-6017' of github.com:ionic-team/ionic-framework into …
thetaPC Mar 15, 2024
6faac2f
Merge branch 'feature-8.0' of github.com:ionic-team/ionic-framework i…
thetaPC Mar 15, 2024
b2ee732
test(many): revert snapshots
thetaPC Mar 18, 2024
dce3c20
Merge branch 'feature-8.0' of github.com:ionic-team/ionic-framework i…
thetaPC Mar 18, 2024
e3682d6
test(many): revert more snapshots
thetaPC Mar 18, 2024
c828936
test(button): revert new screenshots
thetaPC Mar 18, 2024
7edaf5c
refactor(button): use min-width instead
thetaPC Mar 18, 2024
233a951
Update core/src/components/button/button.ios.scss
thetaPC Mar 19, 2024
9f3d1b1
Update core/src/components/button/button.md.scss
thetaPC Mar 19, 2024
2e6a531
refactor(button): update height only for round shapes
thetaPC Mar 19, 2024
5384747
refactor(button): update comment for isCircle
thetaPC Mar 19, 2024
919301a
chore(): add updated snapshots
Ionitron Mar 19, 2024
b3b61f0
Merge branch 'feature-8.0' of github.com:ionic-team/ionic-framework i…
thetaPC Mar 19, 2024
f44ceee
chore(): add updated snapshots
Ionitron Mar 19, 2024
b58a4e2
chore(): add updated snapshots
Ionitron Mar 19, 2024
73aa34e
chore(): add updated snapshots
Ionitron Mar 19, 2024
ae86371
chore(): add updated snapshots
Ionitron Mar 20, 2024
aa02181
chore(): add updated snapshots
Ionitron Mar 20, 2024
de5ab7c
Merge branch 'feature-8.0' of github.com:ionic-team/ionic-framework i…
thetaPC Mar 20, 2024
aee9c7b
refactor(button): use clamp for sizes
thetaPC Mar 20, 2024
0f7febf
refactor(button): add focused state to clear
thetaPC Mar 20, 2024
2029f80
test(button): generate correct clear screenshots
thetaPC Mar 20, 2024
fa6f783
Merge branch 'feature-8.0' of github.com:ionic-team/ionic-framework i…
thetaPC Mar 21, 2024
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
47 changes: 41 additions & 6 deletions core/src/components/button/button.ios.scss
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,6 @@
font-size: #{$button-ios-small-font-size};
}

:host(.button-has-icon-only) {
--padding-top: 0;
--padding-bottom: 0;
}


// iOS Round Button
// --------------------------------------------------
Expand All @@ -131,14 +126,54 @@
--padding-bottom: #{$button-ios-round-padding-bottom};
}


// iOS Strong Button
// --------------------------------------------------

:host(.button-strong) {
font-weight: #{$button-ios-strong-font-weight};
}

// iOS Icon Only Button
// --------------------------------------------------

:host(.button-has-icon-only) {
--padding-top: 0;
--padding-bottom: var(--padding-top);
--padding-end: var(--padding-top);
--padding-start: var(--padding-end);
// TODO(FW-6053): replace em value with the min-height variable.
min-width: clamp(30px, 2.125em, 60px);

// TODO(FW-6053): replace em value with the min-height variable.
min-height: clamp(30px, 2.125em, 60px);
}

::slotted(ion-icon[slot="icon-only"]) {
font-size: 1.125em;
}

:host(.button-small.button-has-icon-only) {
// TODO(FW-6053): replace em value with the min-height variable.
min-width: clamp(23px, 2.16em, 54px);
// TODO(FW-6053): replace em value with the min-height variable.
min-height: clamp(23px, 2.16em, 54px);

}

:host(.button-small) ::slotted(ion-icon[slot="icon-only"]) {
font-size: 1.4em;
}

:host(.button-large.button-has-icon-only) {
// TODO(FW-6053): replace em value with the min-height variable.
min-width: clamp(46px, 2.5em, 78px);
// TODO(FW-6053): replace em value with the min-height variable.
min-height: clamp(46px, 2.5em, 78px);
}

:host(.button-large) ::slotted(ion-icon[slot="icon-only"]) {
font-size: 1em;
}

// iOS Button Focused
// --------------------------------------------------
Expand Down
50 changes: 43 additions & 7 deletions core/src/components/button/button.md.scss
Original file line number Diff line number Diff line change
Expand Up @@ -113,23 +113,59 @@
font-size: #{$button-md-small-font-size};
}

:host(.button-has-icon-only) {
--padding-top: 0;
--padding-bottom: 0;
}


// MD strong Button
// --------------------------------------------------

:host(.button-strong) {
font-weight: #{$button-md-strong-font-weight};
}

// MD Icon Only Button
//
// MD does not specify a small size, these
// styles are based on the iOS small size.
//
// MD does not specify a large size, these
// styles are based on the iOS large size.
// --------------------------------------------------

:host(.button-has-icon-only) {
--padding-top: 0;
--padding-bottom: var(--padding-top);
--padding-end: var(--padding-top);
--padding-start: var(--padding-end);
// TODO(FW-6053): replace em value with the min-height variable.
min-width: clamp(30px, 2.86em, 60px);

// TODO(FW-6053): replace em value with the min-height variable.
min-height: clamp(30px, 2.86em, 60px);
}

::slotted(ion-icon[slot="icon-only"]) {
@include padding(0);
font-size: 1.6em;
}

:host(.button-small.button-has-icon-only) {
// TODO(FW-6053): replace em value with the min-height variable.
min-width: clamp(23px, 2.16em, 54px);
// TODO(FW-6053): replace em value with the min-height variable.
min-height: clamp(23px, 2.16em, 54px);
}

:host(.button-small) ::slotted(ion-icon[slot="icon-only"]) {
font-size: 1.23em;
}

:host(.button-large.button-has-icon-only) {
// TODO(FW-6053): replace em value with the min-height variable.
min-width: clamp(46px, 2.5em, 78px);
// TODO(FW-6053): replace em value with the min-height variable.
min-height: clamp(46px, 2.5em, 78px);
}

:host(.button-large) ::slotted(ion-icon[slot="icon-only"]) {
font-size: 1.4em;
}

// Material Design Button: Hover
// --------------------------------------------------
Expand Down
5 changes: 0 additions & 5 deletions core/src/components/button/button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,6 @@
@include margin(0, -0.2em, 0, 0.3em);
}

::slotted(ion-icon[slot="icon-only"]) {
font-size: 1.8em;
}
Comment on lines -238 to -240
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed since the font size is being overwritten through their respective mode files.



// Button Ripple effect
// --------------------------------------------------

Expand Down
21 changes: 19 additions & 2 deletions core/src/components/button/button.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { ComponentInterface, EventEmitter } from '@stencil/core';
import { Component, Element, Event, Host, Prop, Watch, h } from '@stencil/core';
import { Component, Element, Event, Host, Prop, Watch, State, h } from '@stencil/core';
import type { AnchorInterface, ButtonInterface } from '@utils/element-interface';
import type { Attributes } from '@utils/helpers';
import { inheritAriaAttributes, hasShadowDom } from '@utils/helpers';
Expand Down Expand Up @@ -38,6 +38,11 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf

@Element() el!: HTMLElement;

/**
* If `true`, the button only has an icon.
*/
@State() isCircle: boolean = false;

/**
* The color to use from your application's color palette.
* Default options are: `"primary"`, `"secondary"`, `"tertiary"`, `"success"`, `"warning"`, `"danger"`, `"light"`, `"medium"`, and `"dark"`.
Expand Down Expand Up @@ -295,6 +300,18 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf
this.ionBlur.emit();
};

private slotChanged = () => {
/**
* Ensures that the 'has-icon-only' class is properly added
* or removed from `ion-button` when manipulating the
* `icon-only` slot.
*
* Without this, the 'has-icon-only' class is only checked
* or added when `ion-button` component first renders.
*/
this.isCircle = this.hasIconOnly;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

~ We should explain that we are using a state variable to trigger a re-render of the Stencil component when the slotted content changes, so that has-icon-only class is correctly applied.

I think moving that documentation here to enable the earlier documentation to be more focused on what isCircle is - a variable to track if the button should render as a circle shape or not.

};

render() {
const mode = getIonMode(this);
const {
Expand Down Expand Up @@ -374,7 +391,7 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf
{...inheritedAttributes}
>
<span class="button-inner">
<slot name="icon-only"></slot>
<slot name="icon-only" onSlotchange={this.slotChanged}></slot>
<slot name="start"></slot>
<slot></slot>
<slot name="end"></slot>
Expand Down
2 changes: 1 addition & 1 deletion core/src/components/button/button.vars.scss
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ $button-round-padding-bottom: $button-round-padding-top !default;
$button-round-padding-start: $button-round-padding-end !default;

/// @prop - Border radius of the round button
$button-round-border-radius: 64px !default;
$button-round-border-radius: 999px !default;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the correct value for the large icon only button? The icon is set to 1em which is smaller than the small and default button.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking into this now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brandyscarney this is correct. The large button has a font-size of 20px so the 1em is using that as the base. While the default has a font-size of 16px and small is 13px.

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
60 changes: 56 additions & 4 deletions core/src/components/button/test/round/button.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,64 @@ import { configs, test } from '@utils/test/playwright';
*/
configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
test.describe(title('button: round'), () => {
test('should not have visual regressions', async ({ page }) => {
await page.goto(`/src/components/button/test/round`, config);
test.describe('default', () => {
test('should not have visual regressions', async ({ page }) => {
await page.goto(`/src/components/button/test/round`, config);

await page.setIonViewport();
await page.setIonViewport();

await expect(page).toHaveScreenshot(screenshot(`button-round`));
const container = page.locator('#default');

await expect(container).toHaveScreenshot(screenshot(`button-round`));
});
});

test.describe('outline', () => {
test('should not have visual regressions', async ({ page }) => {
await page.goto(`/src/components/button/test/round`, config);

await page.setIonViewport();

const container = page.locator('#outline');

await expect(container).toHaveScreenshot(screenshot(`button-outline-round`));
});
});

test.describe('clear', () => {
test('should not have visual regressions', async ({ page }) => {
await page.goto(`/src/components/button/test/round`, config);

await page.setIonViewport();

const container = page.locator('#clear');
thetaPC marked this conversation as resolved.
Show resolved Hide resolved

await expect(container).toHaveScreenshot(screenshot(`button-clear-round`));
});
});

test.describe('color', () => {
test('should not have visual regressions', async ({ page }) => {
await page.goto(`/src/components/button/test/round`, config);

await page.setIonViewport();

const container = page.locator('#color');

await expect(container).toHaveScreenshot(screenshot(`button-color-round`));
});
});

test.describe('expand', () => {
test('should not have visual regressions', async ({ page }) => {
await page.goto(`/src/components/button/test/round`, config);

await page.setIonViewport();

const container = page.locator('#expand');

await expect(container).toHaveScreenshot(screenshot(`button-expand-round`));
});
});
});
});
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Loading