Skip to content

Commit

Permalink
fix: Fix Switch not toggling. (#985)
Browse files Browse the repository at this point in the history
  • Loading branch information
stephenh authored Dec 15, 2023
1 parent c2e1bcd commit e2a06bd
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 17 deletions.
1 change: 0 additions & 1 deletion src/inputs/Switch.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ type SwitchWrapperProps = Omit<SwitchProps, "onChange" | "selected"> &

function SwitchWrapper({ isHovered, isFocused, ...props }: SwitchWrapperProps) {
const [selected, setSelected] = useState<boolean>(props.selected || false);

return (
<div
css={{
Expand Down
41 changes: 41 additions & 0 deletions src/inputs/Switch.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { useState } from "react";
import { Switch as SwitchComponent, SwitchProps } from "src/inputs";
import { click, render } from "src/utils/rtl";

describe("Switch", () => {
it("can change", async () => {
const onChange = jest.fn();
// Given a switch
const r = await render(<SwitchTest label="Age" onChange={onChange} />);
// Then it defaults no checked
expect(r.age).not.toBeChecked();
// And when we click it, it flips to checked
click(r.age);
expect(r.age).toBeChecked();
expect(onChange).toHaveBeenCalledTimes(1);
// And if we click it again, it flips back to unchecked
click(r.age);
expect(r.age).not.toBeChecked();
expect(onChange).toHaveBeenCalledTimes(2);
});
});

type SwitchTestProps = Omit<SwitchProps, "onChange" | "selected"> & {
onChange?: (value: boolean) => void;
selected?: boolean;
};

function SwitchTest({ selected: initSelected, onChange: _onChange, ...props }: SwitchTestProps) {
const [selected, setSelected] = useState(initSelected || false);
return (
<SwitchComponent
labelStyle="inline"
selected={selected}
onChange={(value) => {
_onChange?.(value);
setSelected(value);
}}
{...props}
/>
);
}
36 changes: 20 additions & 16 deletions src/inputs/Switch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { resolveTooltip } from "src/components";
import { Label } from "src/components/Label";
import { Css, Palette } from "src/Css";
import { Icon } from "../components/Icon";
import { toToggleState } from "../utils";
import { toToggleState, useTestIds } from "../utils";

export interface SwitchProps {
/** Whether the element should receive focus on render. */
Expand All @@ -17,7 +17,7 @@ export interface SwitchProps {
label: string;
/** Where to put the label. */
labelStyle?: "form" | "inline" | "filter" | "hidden" | "left" | "centered"; // TODO: Update `labelStyle` to make consistent with other `labelStyle` properties in the library
/** Whether or not to hide the label */
/** Whether to hide the label */
hideLabel?: boolean;
/** Handler when the interactive element state changes. */
onChange: (value: boolean) => void;
Expand Down Expand Up @@ -49,12 +49,13 @@ export function Switch(props: SwitchProps) {
const { isFocusVisible: isKeyboardFocus, focusProps } = useFocusRing(otherProps);
const { hoverProps, isHovered } = useHover(ariaProps);
const tooltip = resolveTooltip(disabled, props.tooltip);
const tid = useTestIds(otherProps, label);

return (
<div
<label
{...hoverProps}
css={{
...Css.relative.cursorPointer.df.w("max-content").selectNone.$,
...Css.relative.cursorPointer.df.wmaxc.selectNone.$,
...(labelStyle === "form" && Css.fdc.$),
...(labelStyle === "left" && Css.w100.aic.$),
...(labelStyle === "inline" && Css.gap2.aic.$),
Expand Down Expand Up @@ -105,9 +106,9 @@ export function Switch(props: SwitchProps) {
<Label label={label} tooltip={tooltip} inline xss={Css.smMd.gray900.if(compact).add("lineHeight", "1").$} />
)}
<VisuallyHidden>
<input ref={ref} {...inputProps} {...focusProps} />
<input ref={ref} {...inputProps} {...focusProps} {...tid} />
</VisuallyHidden>
</div>
</label>
);
}

Expand All @@ -122,22 +123,25 @@ export const switchFocusStyles = Css.bshFocus.$;
export const switchSelectedHoverStyles = Css.bgBlue900.$;

// Circle inside Switcher/Toggle element styles
const switchCircleDefaultStyles = (isCompact: boolean) => ({
...Css.wPx(circleDiameter(isCompact))
.hPx(circleDiameter(isCompact))
.br100.bgWhite.bshBasic.absolute.leftPx(2)
.topPx(2).transition.df.aic.jcc.$,
svg: Css.hPx(toggleHeight(isCompact) / 2).wPx(toggleHeight(isCompact) / 2).$,
});
function switchCircleDefaultStyles(isCompact: boolean) {
return {
...Css.wPx(circleDiameter(isCompact))
.hPx(circleDiameter(isCompact))
.br100.bgWhite.bshBasic.absolute.leftPx(2)
.topPx(2).transition.df.aic.jcc.$,
svg: Css.hPx(toggleHeight(isCompact) / 2).wPx(toggleHeight(isCompact) / 2).$,
};
}

/**
* Affecting the `left` property due to transitions only working when there is
* a previous value to work from.
*
* Calculation is as follow:
* Calculation is as follows:
* - `100%` is the toggle width
* - `${circleDiameter(isCompact)}px` is the circle diameter
* - `2px` is to keep 2px edge spacing.
*/
const switchCircleSelectedStyles = (isCompact: boolean) =>
Css.left(`calc(100% - ${circleDiameter(isCompact)}px - 2px);`).$;
function switchCircleSelectedStyles(isCompact: boolean) {
return Css.left(`calc(100% - ${circleDiameter(isCompact)}px - 2px);`).$;
}

0 comments on commit e2a06bd

Please sign in to comment.