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(pie-button): DSW-000 styling allowing for text-overflow in buttons #1089

Merged
merged 4 commits into from
Nov 29, 2024
Merged
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
5 changes: 5 additions & 0 deletions .changeset/clever-trees-sort.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@justeattakeaway/pie-button": minor
---

[Changed] - Styling allowing for text-overflow in buttons
5 changes: 5 additions & 0 deletions .changeset/rotten-chefs-divide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@justeattakeaway/pie-button": minor
---

[Changed] - enabled text wrapping onto multiple lines for all button types
128 changes: 108 additions & 20 deletions packages/components/pie-button/src/button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -25,59 +25,83 @@
// currently this sets the primary button styles
--btn-bg-color: var(--dt-color-interactive-brand);
--btn-text-color: var(--dt-color-content-interactive-primary);

// Heights for the different button sizes
--btn-height--xsmall: 32px;
--btn-height--small: 40px;
--btn-height--medium: 48px;
--btn-height--large: 56px;
--icon-display-override: block;

// Vertical and horizontal padding values for the button
--btn-padding-vertical-xsmall: 6px;
--btn-padding-vertical-small: 8px;
--btn-padding-vertical-medium: 10px;
--btn-padding-vertical-large: 14px;
--btn-padding-horizontal-small: var(--dt-spacing-b);
--btn-padding-horizontal-medium: var(--dt-spacing-d);
--btn-padding-horizontal-large: var(--dt-spacing-e);

/**
* Mixin for updating the button styles based on the size passed in.
* Takes in the name of the size to be used.
*/
@mixin button-size($size) {
@if $size == 'xsmall' {
--btn-height: var(--btn-height--xsmall);
--btn-padding: 6px var(--dt-spacing-b);
--btn-font-size: #{p.font-size(--dt-font-size-14)};
--btn-line-height: calc(var(--dt-font-size-14-line-height) * 1px);
--icon-size-override: 16px;
} @else if $size == 'small-expressive' {
--btn-height: var(--btn-height--small);
--btn-padding: 6px var(--dt-spacing-d);
--btn-font-size: #{p.font-size(--dt-font-size-20)};
--btn-line-height: calc(var(--dt-font-size-20-line-height) * 1px);
--icon-size-override: 20px;
} @else if $size == 'small-productive' {
--btn-height: var(--btn-height--small);
--btn-padding: 8px var(--dt-spacing-d);
--btn-font-size: #{p.font-size(--dt-font-size-16)};
--btn-line-height: calc(var(--dt-font-size-16-line-height) * 1px);
--icon-size-override: 20px;
} @else if $size == 'medium' {
--btn-height: var(--btn-height--medium);
--btn-padding: 10px var(--dt-spacing-e);
--btn-font-size: #{p.font-size(--dt-font-size-20)};
--btn-line-height: calc(var(--dt-font-size-20-line-height) * 1px);
--icon-size-override: 24px;
} @else if $size == 'large' {
--btn-height: var(--btn-height--large);
--btn-padding: 14px var(--dt-spacing-e);
--btn-font-size: #{p.font-size(--dt-font-size-20)};
--btn-line-height: calc(var(--dt-font-size-20-line-height) * 1px);
--icon-size-override: 24px;
}
}

/**
* Mixin for applying the button padding based on the size passed in.
* $size: The name of the size to be used.
* $offsetPaddingForBorder: A boolean to specify whether the padding should be offset for a border
*/
@mixin button-padding($size, $offsetPaddingForBorder: false) {
$verticalPadding: '';
$horizontalPadding: '';

@if $size == 'xsmall' {
$verticalPadding: '--btn-padding-vertical-xsmall';
$horizontalPadding: '--btn-padding-horizontal-small';
} @else if $size == 'small-expressive' {
$verticalPadding: '--btn-padding-vertical-xsmall';
$horizontalPadding: '--btn-padding-horizontal-medium';
} @else if $size == 'small-productive' {
$verticalPadding: '--btn-padding-vertical-small';
$horizontalPadding: '--btn-padding-horizontal-medium';
} @else if $size == 'medium' {
$verticalPadding: '--btn-padding-vertical-medium';
$horizontalPadding: '--btn-padding-horizontal-large';
} @else if $size == 'large' {
$verticalPadding: '--btn-padding-vertical-large';
$horizontalPadding: '--btn-padding-horizontal-large';
}

@if $offsetPaddingForBorder {
padding: calc(var(#{$verticalPadding}) - 1px) var(#{$horizontalPadding});
} @else {
padding: var(#{$verticalPadding}) var(#{$horizontalPadding});
}
}

position: relative;
display: inline-flex;
gap: var(--dt-spacing-b);
align-items: center;
justify-content: center;
height: var(--btn-height);
padding: var(--btn-padding);
border: none;
border-radius: var(--btn-border-radius);
outline: none;
Expand Down Expand Up @@ -167,7 +191,7 @@
@include p.button-interactive-states('--dt-color-container-default', 'transparent');
}

&.o-btn--outline-inverse:not([disabled]) {
&.o-btn--outline-inverse {
border: 1px solid var(--dt-color-border-strong);
}

Expand All @@ -191,43 +215,98 @@
// *********************
&.o-btn--xsmall {
@include button-size(xsmall);
@include button-padding(xsmall);

@include responsive-wide {
// productive is the default size when responsive is enabled
@include button-size(small-productive);
@include button-padding(small-productive);

&.o-btn--expressive {
@include button-size(small-expressive);
@include button-padding(small-expressive);
}
}

&.o-btn--outline,
&.o-btn--outline-inverse {
@include button-padding(xsmall, true);

@include responsive-wide {
@include button-padding(small-productive, true);

&.o-btn--expressive {
@include button-padding(small-expressive, true);
}
}
}
}

&.o-btn--small-expressive {
@include button-size(small-expressive);
@include button-padding(small-expressive);

@include responsive-wide {
@include button-size(medium);
@include button-padding(medium);
}

&.o-btn--outline,
&.o-btn--outline-inverse {
@include button-padding(small-expressive, true);

@include responsive-wide {
@include button-padding(medium, true);
}
}
}

&.o-btn--small-productive {
@include button-size(small-productive);
@include button-padding(small-productive);

@include responsive-wide {
@include button-size(medium);
@include button-padding(medium);
}

&.o-btn--outline,
&.o-btn--outline-inverse {
@include button-padding(small-productive, true);

@include responsive-wide {
@include button-padding(medium, true);
}
}
}

&.o-btn--medium {
@include button-size(medium);
@include button-padding(medium);

@include responsive-wide {
@include button-size(large);
@include button-padding(large);
}

&.o-btn--outline,
&.o-btn--outline-inverse {
@include button-padding(medium, true);

@include responsive-wide {
@include button-padding(large, true);
}
}
}

&.o-btn--large {
@include button-size(large);
@include button-padding(large);

&.o-btn--outline,
&.o-btn--outline-inverse {
@include button-padding(large, true);
}
}

// ******************************
Expand All @@ -251,7 +330,8 @@
}

// For outline variants, set the border to the disabled color
&.o-btn--outline {
&.o-btn--outline,
&.o-btn--outline-inverse {
border-color: var(--dt-color-disabled-01) !important;
}
}
Expand All @@ -277,6 +357,14 @@
}
}

// The inner span for the button text is needed to handle the text-overflow
// Without this, single word overflow wouldn't be possible in combination
// with display: flex.
.o-btn-text {
text-overflow: ellipsis;
raoufswe marked this conversation as resolved.
Show resolved Hide resolved
overflow: hidden;
}

// Used to size an SVG if one is passed in (when not using the component icons)
::slotted(svg) {
height: var(--icon-size-override);
Expand Down
2 changes: 1 addition & 1 deletion packages/components/pie-button/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ export class PieButton extends FormControlMixin(LitElement) implements ButtonPro
?disabled=${disabled}>
${isLoading ? this.renderSpinner() : nothing}
${iconPlacement === 'leading' ? html`<slot name="icon"></slot>` : nothing}
<slot></slot>
<span class="o-btn-text"><slot></slot></span>
${iconPlacement === 'trailing' ? html`<slot name="icon"></slot>` : nothing}
</button>`;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const props: PropObject<ButtonProps> = {

// Renders a <pie-button> HTML string with the given prop values
const renderTestPieButton = (propVals: WebComponentPropValues) => `<pie-button size="${propVals.size}" ${propVals.isResponsive ? 'isResponsive' : ''} responsiveSize="${propVals.responsiveSize ? propVals.responsiveSize : ''}">Hello world</pie-button>`;
const renderTestPieButtonLargeText = (propVals: WebComponentPropValues) => `<pie-button size="${propVals.size}" ${propVals.isResponsive ? 'isResponsive' : ''} responsiveSize="${propVals.responsiveSize ? propVals.responsiveSize : ''}">This is a really long button string to test the overflow</pie-button>`;

const componentPropsMatrix = getAllPropCombinations(props);

Expand Down Expand Up @@ -55,3 +56,25 @@ test('should render all size variations', async ({ page, mount }) => {

await percySnapshot(page, 'PIE Button - sizes/isResponsive/responsiveSize', percyWidths);
});

test('should render all size variations, with larger button text string', async ({ page, mount }) => {
for (const combo of componentPropsMatrix) {
const testComponent = createTestWebComponent(combo, renderTestPieButtonLargeText);
const propKeyValues = `size: ${testComponent.propValues.size}, isResponsive: ${testComponent.propValues.isResponsive}, responsiveSize: ${testComponent.propValues.responsiveSize}`;

await mount(
WebComponentTestWrapper,
{
props: { propKeyValues },
slots: {
component: testComponent.renderedString.trim(),
},
},
);
}

// Follow up to remove in Jan
await page.waitForTimeout(2500);

await percySnapshot(page, 'PIE Button - sizes/isResponsive/responsiveSize - double line text', percyWidths);
});
Loading