Skip to content

Commit

Permalink
Pill next updates (#2903)
Browse files Browse the repository at this point in the history
  • Loading branch information
Fercas123 authored Jan 12, 2024
1 parent 6481f62 commit 641197d
Show file tree
Hide file tree
Showing 11 changed files with 65 additions and 204 deletions.
6 changes: 6 additions & 0 deletions .changeset/pretty-poets-appear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@salt-ds/lab": minor
---

- Removed `onClose` prop from `PillNext`. PillNext has been updated to support only one action. The `onClick`prop can be used instead.
- Remove `icon` prop from `PillNext`. An icon can be added as a children instead.
49 changes: 0 additions & 49 deletions packages/lab/src/__tests__/__e2e__/pill-next/PillNext.cy.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { PillNext } from "../../../pill-next";
import { CallIcon } from "@salt-ds/icons";

describe("GIVEN a Pill", () => {
it("THEN should render a `standard` Pill", () => {
Expand Down Expand Up @@ -50,54 +49,6 @@ describe("GIVEN a Pill", () => {
});
});

describe("GIVEN an icon prop", () => {
it("THEN should render an icon given icon component", () => {
cy.mount(<PillNext icon={<CallIcon />}>label</PillNext>);
cy.findByTestId(/CallIcon/i).should("exist");
});
});

describe("GIVEN a closable pill", () => {
it("THEN should not trigger close by clicking the pill", () => {
const clickSpy = cy.stub().as("clickSpy");
const closeSpy = cy.stub().as("closeSpy");
cy.mount(
<PillNext onClick={clickSpy} onClose={closeSpy}>
Closable Pill
</PillNext>
);
cy.findByTestId("pill").realClick();
cy.get("@clickSpy").should("have.callCount", 1);
cy.get("@closeSpy").should("have.callCount", 0);
});
it("THEN should close the pill on clicking the close button", () => {
const clickSpy = cy.stub().as("clickSpy");
const closeSpy = cy.stub().as("closeSpy");
cy.mount(
<PillNext onClick={clickSpy} onClose={closeSpy}>
Closable Pill
</PillNext>
);
cy.findByTestId("pill-close-button").realClick();
cy.get("@clickSpy").should("have.callCount", 0);
cy.get("@closeSpy").should("have.callCount", 1);
});
it("THEN should close on enter", () => {
const closeSpy = cy.stub().as("closeSpy");
cy.mount(<PillNext onClose={closeSpy}>Closable Pill</PillNext>);
cy.findByTestId("pill-close-button").focus();
cy.realPress("Enter");
cy.get("@closeSpy").should("have.callCount", 1);
});
it("THEN should close on space", () => {
const closeSpy = cy.stub().as("closeSpy");
cy.mount(<PillNext onClose={closeSpy}>Closable Pill</PillNext>);
cy.findByTestId("pill-close-button").focus();
cy.realPress(" ");
cy.get("@closeSpy").should("have.callCount", 1);
});
});

it("SHOULD have no a11y violations on load", () => {
const clickSpy = cy.stub().as("clickSpy");
cy.mount(<PillNext onClick={clickSpy}>Pill</PillNext>);
Expand Down
65 changes: 12 additions & 53 deletions packages/lab/src/pill-next/PillNext.css
Original file line number Diff line number Diff line change
@@ -1,27 +1,9 @@
/* Styles applied to the root element */

.saltPillNext {
display: flex;
max-width: 100%;
}

.saltPillNext-action {
--pillNext-background: var(--salt-actionable-primary-background);
--pillNext-background-active: var(--salt-actionable-primary-background-active);
--pillNext-background-disabled: var(--salt-actionable-primary-background-disabled);
--pillNext-background-hover: var(--salt-actionable-primary-background-hover);
--pillNext-text-color: var(--salt-actionable-primary-foreground);
--pillNext-text-color-active: var(--salt-actionable-primary-foreground-active);
--pillNext-text-color-hover: var(--salt-actionable-primary-foreground-hover);
--pillNext-text-color-disabled: var(--salt-actionable-primary-foreground-disabled);
}

.saltPillNext-action {
appearance: none;
-webkit-appearance: none;
display: inline-flex;
align-items: center;
background: var(--pillNext-background);
background: var(--saltPillNext-background, var(--salt-actionable-primary-background));
border-radius: 0;
border: 0;
height: calc(var(--salt-size-base) - var(--salt-spacing-100));
Expand All @@ -31,64 +13,41 @@
gap: var(--salt-spacing-50);
padding-left: var(--salt-spacing-50);
padding-right: var(--salt-spacing-50);
color: var(--pillNext-text-color);
color: var(--saltPillNext-color, var(--salt-actionable-primary-foreground));
font-family: var(--salt-text-fontFamily);
font-size: var(--salt-text-fontSize);
font-weight: var(--salt-text-fontWeight);
line-height: var(--salt-text-lineHeight);
}

.saltPillNext-label {
min-width: 0;
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
}

.saltPillNext-close-button {
--saltButton-height: calc(var(--salt-size-base) - var(--salt-spacing-100));
--saltButton-padding: var(--salt-spacing-50);
}

/* Style applied to Pill if `onClick` prop is provided */
/* Style applied to Pill if pill is clickable */
.saltPillNext-clickable {
cursor: var(--salt-selectable-cursor-hover);
}

.saltPillNext-clickable:hover,
.saltPillNext-clickable:focus-visible {
color: var(--pillNext-text-color-hover);
background: var(--pillNext-background-hover);
}

.saltPillNext-clickable.saltPillNext-disabled:hover {
color: var(--pillNext-text-color);
background: var(--pillNext-background);
color: var(--salt-actionable-primary-foreground-hover);
background: var(--salt-actionable-primary-background-hover);
}

.saltPillNext-clickable.saltPillNext-active,
.saltPillNext-clickable:active {
background: var(--pillNext-background-active);
color: var(--pillNext-text-color-active);
}

.saltPillNext-clickable.saltPillNext-disabled.saltPillNext-active,
.saltPillNext-clickable.saltPillNext-disabled:active {
background: var(--pillNext-background);
color: var(--pillNext-text-color);
background: var(--salt-actionable-primary-background-active);
color: var(--salt-actionable-primary-foreground-active);
}

/* Style applied to Pill on focus */
.saltPillNext-action:focus-visible {
.saltPillNext:focus-visible {
outline: var(--salt-focused-outline);
/* increase index by one so the focus ring sits on top of the sibling button */
z-index: var(--salt-zIndex-default);
}

/* Style applied to Pill when disabled */
.saltPillNext-action.saltPillNext-disabled,
.saltPillNext-action.saltPillNext-disabled:hover {
color: var(--pillNext-text-color-disabled);
background: var(--pillNext-background-disabled);
.saltPillNext:disabled,
.saltPillNext:disabled:hover {
color: var(--salt-actionable-primary-foreground-disabled);
background: var(--salt-actionable-primary-background-disabled);
cursor: var(--salt-selectable-cursor-disabled);
}
70 changes: 32 additions & 38 deletions packages/lab/src/pill-next/PillNext.tsx
Original file line number Diff line number Diff line change
@@ -1,24 +1,27 @@
import { forwardRef, ComponentPropsWithoutRef, MouseEvent } from "react";
import { forwardRef, ComponentPropsWithoutRef } from "react";
import clsx from "clsx";
import { useWindow } from "@salt-ds/window";
import { useComponentCssInjection } from "@salt-ds/styles";
import { Button, makePrefixer, useButton } from "@salt-ds/core";
import { makePrefixer, useButton } from "@salt-ds/core";
import pillCss from "./PillNext.css";
import { CloseIcon } from "@salt-ds/icons";

export interface PillNextProps extends ComponentPropsWithoutRef<"button"> {
/* If true the pill will be disabled */
disabled?: boolean;
onClose?: (event: MouseEvent<HTMLButtonElement>) => void;
/* Pass an element to render an icon descriptor on the left of the label */
icon?: React.ReactNode;
}

const withBaseName = makePrefixer("saltPillNext");

/* eslint-disable @typescript-eslint/no-empty-interface */
export interface PillNextProps extends ComponentPropsWithoutRef<"button"> {}

export const PillNext = forwardRef<HTMLButtonElement, PillNextProps>(
function PillNext(
{ children, className, icon, disabled, onClose, ...restProps },
{
children,
className,
disabled,
onKeyUp,
onKeyDown,
onClick,
onBlur,
...rest
},
ref
) {
const targetWindow = useWindow();
Expand All @@ -29,39 +32,30 @@ export const PillNext = forwardRef<HTMLButtonElement, PillNextProps>(
});
const { buttonProps, active } = useButton<HTMLButtonElement>({
disabled,
...restProps,
onKeyUp,
onKeyDown,
onClick,
onBlur,
});
// we do not want to spread tab index in this case because the button element
// does not require tabindex="0" attribute
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { tabIndex, ...restButtonProps } = buttonProps;

return (
<div className={clsx(withBaseName(), className)}>
<button
data-testid="pill"
ref={ref}
className={clsx(withBaseName("action"), withBaseName("clickable"), {
[withBaseName("active")]: active,
[withBaseName("disabled")]: disabled,
})}
{...restButtonProps}
{...restProps}
>
{icon}
<span className={withBaseName("label")}>{children}</span>
</button>
{onClose && (
<Button
data-testid="pill-close-button"
className={withBaseName("close-button")}
disabled={disabled}
onClick={onClose}
>
<CloseIcon />
</Button>
<button
data-testid="pill"
ref={ref}
className={clsx(
withBaseName(),
withBaseName("clickable"),
{ [withBaseName("active")]: active },
className
)}
</div>
{...restButtonProps}
{...rest}
>
{children}
</button>
);
}
);
10 changes: 2 additions & 8 deletions packages/lab/stories/pill-next/pill-next.qa.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,8 @@ export const ExamplesGrid: StoryFn<QAContainerProps> = (props) => {
<PillNext className={className} onClick={noop} disabled>
Disabled Pill
</PillNext>
<PillNext className={className} icon={<FavoriteIcon />} onClick={noop}>
With Icon Pill
</PillNext>
<PillNext onClose={noop} className={className}>
Closable Pill
</PillNext>
<PillNext onClick={noop} className={className}>
Extra extra long Pill label example.
<PillNext className={className} onClick={noop}>
<FavoriteIcon /> With Icon Pill
</PillNext>
</QAContainer>
);
Expand Down
49 changes: 7 additions & 42 deletions packages/lab/stories/pill-next/pill-next.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { PillNext } from "@salt-ds/lab";
import { FavoriteIcon } from "@salt-ds/icons";
import { CloseIcon, FavoriteIcon } from "@salt-ds/icons";
import { Meta, StoryFn } from "@storybook/react";
import { ChangeEvent, Ref, useEffect, useRef, useState } from "react";
import { useState } from "react";
import { shortColorData } from "./../assets/exampleData";
import { Button, FlowLayout, Input, StackLayout, Tooltip } from "@salt-ds/core";
import { Button, FlowLayout } from "@salt-ds/core";

export default {
title: "Lab/Pill Next",
Expand Down Expand Up @@ -48,10 +48,9 @@ export const Closable: StoryFn<typeof PillNext> = () => {
<PillNext
key={color}
disabled={index < 3}
onClick={() => console.log(`Clicked ${color}`)}
onClose={() => removeColor(color)}
onClick={() => removeColor(color)}
>
{color}
{color} <CloseIcon />
</PillNext>
))}
</FlowLayout>
Expand All @@ -60,42 +59,8 @@ export const Closable: StoryFn<typeof PillNext> = () => {

export const Icon: StoryFn<typeof PillNext> = () => {
return (
<PillNext icon={<FavoriteIcon />} onClick={() => console.log("Clicked.")}>
Pill with Icon
<PillNext onClick={() => console.log("Clicked.")}>
<FavoriteIcon /> Pill with Icon
</PillNext>
);
};

export const Truncated: StoryFn<typeof PillNext> = () => {
const pillRef = useRef<HTMLButtonElement | null>(null);
const [maxWidth, setMaxWidth] = useState<string>("150");
const [isEllipsisActive, setEllipsisActive] = useState(false);

useEffect(() => {
const text = pillRef?.current?.firstElementChild as HTMLElement;
setEllipsisActive(text?.offsetWidth < text?.scrollWidth);
}, [maxWidth]);

const content = "Lorem ipsum dolor sit amet, consectetur adipiscing elit.";

return (
<StackLayout direction={"column"}>
<Input
inputProps={{ type: "number", step: 50 }}
style={{ maxWidth: "150px" }}
defaultValue={maxWidth}
onChange={(event: ChangeEvent<HTMLInputElement>) =>
setMaxWidth(event.target.value)
}
/>
<Tooltip content={content} disabled={!isEllipsisActive}>
<PillNext
ref={pillRef as Ref<HTMLButtonElement>}
style={{ maxWidth: `${maxWidth}px` }}
>
{content}
</PillNext>
</Tooltip>
</StackLayout>
);
};
3 changes: 0 additions & 3 deletions site/docs/components/pill/accessibility.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,15 @@ data:
<KeyboardControls>
<KeyboardControl keyOrCombos="Enter / Space">
- When focus is on the pill, this action activates it.
- When focus is on the close button, this action closes the pill.
</KeyboardControl>
<KeyboardControl keyOrCombos="Tab">

- Tab sets focus on the entire pill.
- When the pill has focus, Tab moves focus out of the pill to next component in tab order.
- When a closable pill has focus, Tab moves focus into the close button.

</KeyboardControl>
<KeyboardControl keyOrCombos="Shift + Tab">
- When the pill has focus, this action moves focus out of the pill to the previous component in the tab
order.
- When the close button has focus, this action moves focus back to the pill.
</KeyboardControl>
</KeyboardControls>
7 changes: 0 additions & 7 deletions site/docs/components/pill/examples.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,5 @@ data:

The closable pill represents a value selected via another mechanism, and which the user can remove within the pill. It can appear on its own, such as alongside a complex filter panel, or within an input control such as [`ComboBox`](../combo-box).

</LivePreview>
<LivePreview componentName="pill" exampleName="Closable">

### Closable

The closable pill represents a value selected via another mechanism, and which the user can remove within the pill. It can appear on its own, such as alongside a complex filter panel, or within an input control such as [`ComboBox`](../combo-box).

</LivePreview>
</LivePreviewControls>
1 change: 0 additions & 1 deletion site/docs/components/pill/index.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ data:
[
{ name: "Button", relationship: "similarTo" },
{ name: "Toggle button", relationship: "similarTo" },
{ name: "Icon", relationship: "contains" },
]
stickerSheet: ""
layout: DetailComponent
Expand Down
Loading

1 comment on commit 641197d

@vercel
Copy link

@vercel vercel bot commented on 641197d Jan 12, 2024

Choose a reason for hiding this comment

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

Please sign in to comment.