Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ declare const classNames: {
readonly stateIcon: "stateIcon";
readonly icon: "icon";
readonly text: "text";
readonly hasText: "hasText";
readonly counterBadge: "counterBadge";
readonly isPending: "isPending";
readonly isFailed: "isFailed";
readonly isSucceeded: "isSucceeded";
readonly plain: "plain";
readonly "size-s": "size-s";
readonly primary: "primary";
readonly solid: "solid";
Expand All @@ -21,6 +21,7 @@ declare const classNames: {
readonly light: "light";
readonly "dark-static": "dark-static";
readonly "light-static": "light-static";
readonly plain: "plain";
readonly soft: "soft";
readonly avatar: "avatar";
readonly image: "image";
Expand Down
16 changes: 4 additions & 12 deletions packages/components/src/components/Button/Button.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
column-gap: var(--button--spacing);
align-items: center;
justify-content: center;
text-align: start;
text-wrap-style: pretty;
}

.content,
Expand All @@ -51,7 +53,7 @@
justify-self: center;
}

&:where(:has(.icon):not(:has(.text))) {
&:where(:has(.icon):not(:has(.text), .hasText)) {
padding: var(--button--padding-icon-only);

&:where(.outline) {
Expand All @@ -61,10 +63,6 @@
}
}

&:where(:has(.text) .icon) {
margin-inline-start: var(--button--spacing);
}

.counterBadge {
position: absolute;
top: calc(var(--size-px--xxs) * -1);
Expand All @@ -79,12 +77,6 @@
.content {
opacity: 0;
}

&.plain {
.content:has(.text) {
opacity: 1;
}
}
}

&.isPending {
Expand Down Expand Up @@ -112,7 +104,7 @@
);
}

&:where(:has(.icon):not(:has(.text))) {
&:where(:has(.icon):not(:has(.text), .hasText)) {
padding: var(--button--padding-s-icon-only);

&:where(.outline) {
Expand Down
18 changes: 9 additions & 9 deletions packages/components/src/components/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@ import type { PropsContext } from "@/lib/propsContext";
import { PropsContextProvider } from "@/lib/propsContext";
import { IconFailed, IconSucceeded } from "@/components/Icon/components/icons";
import { Wrap } from "@/components/Wrap";
import { Text } from "@/components/Text";
import type { FlowComponentProps } from "@/lib/componentFactory/flowComponent";
import { flowComponent } from "@/lib/componentFactory/flowComponent";
import LoadingSpinner from "@/components/LoadingSpinner/LoadingSpinner";
import { useAriaAnnounceActionState } from "@/components/Action/lib/ariaLive";
import { extractTextFromFirstChild } from "@/lib/react/remote";
import { containsTextChild } from "@/lib/react/remote";
import type { AlphaColor } from "@/lib/types/props";
import { filterDOMProps } from "@react-aria/utils";
import { useWarnDeprecation } from "@/components/DeprecationWarningProvider";
Expand Down Expand Up @@ -130,6 +129,12 @@ export const Button = flowComponent("Button", (props) => {

const color = colorFromProps === "accent" ? "success" : colorFromProps;

/**
* A string child renders no element the CSS could match, so the button itself
* carries the marker. An explicit `Text` child is matched by `:has(.text)`.
*/
const hasText = containsTextChild(children);

const rootClassName = unstyled
? className
: clsx(
Expand All @@ -145,6 +150,7 @@ export const Button = flowComponent("Button", (props) => {
* by now, so this Button will be visually disabled via CSS.
*/
ariaDisabled && styles.ariaDisabled,
hasText && styles.hasText,
className,
);

Expand Down Expand Up @@ -186,17 +192,11 @@ export const Button = flowComponent("Button", (props) => {
<LoadingSpinner size={size} className={styles.stateIcon} />
) : undefined;

const isStringContent = extractTextFromFirstChild(children) !== undefined;

const content = (
<>
<PropsContextProvider props={propsContext}>
<Wrap if={!unstyled}>
<span className={styles.content}>
<Wrap if={isStringContent}>
<Text className={styles.text}>{children}</Text>
</Wrap>
</span>
<span className={styles.content}>{children}</span>
</Wrap>
</PropsContextProvider>
{stateIcon}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
ComponentUsageProvider,
type ComponentUsageEvent,
} from "@/components/ComponentUsageProvider";
import { AlertBadge } from "@/components/AlertBadge";
import { Button } from "@/components/Button";
import { Heading } from "@/components/Heading";
import { Section } from "@/components/Section";
Expand Down Expand Up @@ -44,12 +45,12 @@ test("reports the components that render", async () => {

test("over-reports a composition that bypasses its view", async () => {
// Known limitation, pinned on purpose: the exclusion sits at the view seam, so
// Button rendering <Text> directly instead of TextView lands in the consumer's
// bucket. Polling on Button is the flush gate — React runs the child's mount
// effect first, so once Button is in, Text is too.
const events = await renderCollecting(<Button>Fire</Button>);
// AlertBadge rendering <Text> directly instead of TextView lands in the
// consumer's bucket. Polling on AlertBadge is the flush gate — React runs the
// child's mount effect first, so once AlertBadge is in, Text is too.
const events = await renderCollecting(<AlertBadge>Fire</AlertBadge>);

await expect.poll(() => componentsOf(events)).toContain("Button");
await expect.poll(() => componentsOf(events)).toContain("AlertBadge");
expect(componentsOf(events)).toContain("Text");
});

Expand Down
3 changes: 1 addition & 2 deletions packages/components/src/components/Link/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
type PropsWithClassName,
} from "@/lib/types/props";
import { linkContext } from "@/components/Link/context";
import { Text } from "@/components/Text";
import { LinkIcon } from "@/components/Link/components/LinkIcon";
import { handleLinkClick, useRouter } from "@react-aria/utils";
import { UiComponentTunnelExit } from "@/components/UiComponentTunnel/UiComponentTunnelExit";
Expand Down Expand Up @@ -126,7 +125,7 @@ export const Link = flowComponent("Link", (props) => {
isDisabled: props.isDisabled,
children: dynamic((buttonProps) => (
<>
<Text>{buttonProps.children}</Text>
{buttonProps.children}
<LinkIcon download={download} target={target} unstyled={unstyled} />
</>
)),
Expand Down
27 changes: 26 additions & 1 deletion packages/components/src/lib/react/remote.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,13 @@ import type {
RemoteTextRendererProps,
} from "@mittwald/remote-dom-react/host";
import { isObjectType, isString } from "remeda";
import { Children, isValidElement, type ReactNode } from "react";
import {
Children,
Fragment,
isValidElement,
type PropsWithChildren,
type ReactNode,
} from "react";

export function isRemoteComponentRendererProps(
props: unknown,
Expand All @@ -16,6 +22,25 @@ export function isRemoteComponentRendererProps(
);
}

/** Whether the node renders text — a string, number, or remote counterpart. */
const isTextNode = (child: ReactNode): boolean =>
typeof child === "string" ||
typeof child === "number" ||
(isValidElement(child) && isRemoteTextRenderProps(child.props));

/**
* `Children.toArray` treats a fragment as a single child, so recurse into one
* to reach the text a component like `Link` passes down through it.
*/
export const containsTextChild = (children: ReactNode): boolean =>
Children.toArray(children).some(
(child) =>
isTextNode(child) ||
(isValidElement<PropsWithChildren>(child) &&
child.type === Fragment &&
containsTextChild(child.props.children)),
);

export const extractTextFromFirstChild = (children: ReactNode) => {
if (Children.count(children) !== 1) {
return undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,11 @@ test.each(testEnvironments)(

test.each(testEnvironments)(
"Link with Button (%s)",
async ({ testScreenshot, render, components: { Link, Flex, Button } }) => {
async ({
testScreenshot,
render,
components: { Link, Flex, Button, IconStar },
}) => {
await render(
<Flex gap="m" direction="column">
<Link target="_blank">
Expand All @@ -96,6 +100,17 @@ test.each(testEnvironments)(
<Link isDisabled>
<Button>Disabled</Button>
</Link>
<Link target="_blank">
<Button>
<IconStar />
Icon & Text
</Button>
</Link>
<Link target="_blank">
<Button aria-label="Icon only">
<IconStar />
</Button>
</Link>
</Flex>,
);

Expand Down
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