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.

Sign up for GitHub

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

Card: refactor component for improved accessibility #4574

Merged
merged 1 commit into from
Jan 21, 2025
Merged

Card: refactor component for improved accessibility #4574

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 19 additions & 1 deletion packages/orbit-components/src/Card/Card.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,24 @@ export const CardWithDefaultExpanded: Story = {
>
Hidden content
</CardSection>
<CardSection
expandable
header={
<Stack inline align="center" justify="end">
<Text type="secondary">Trip length: 1h55m</Text>
<Badge icon={<Clock />} type="warningSubtle">
Unavailable
</Badge>
</Stack>
}
actions={
<ButtonLink compact type="secondary" size="small">
Open
</ButtonLink>
}
>
Hidden content
</CardSection>
<CardSection
expandable
initialExpanded={initialExpanded}
Expand All @@ -269,7 +287,7 @@ export const CardWithDefaultExpanded: Story = {
}
onClose={action("onClose")}
header={
<Stack inline justify="end">
<Stack inline align="center" justify="end">
<Text type="secondary">Trip length: 1h55m</Text>
<Badge icon={<Clock />} type="warningSubtle">
Unavailable
Expand Down
53 changes: 36 additions & 17 deletions packages/orbit-components/src/Card/CardSection/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ import type { Props } from "./types";
import Header from "../components/Header";
import Expandable from "./components/Expandable";
import handleKeyDown from "../../utils/handleKeyDown";
import Stack from "../../Stack";

const Actions = ({ actions }) => (
Copy link
Member Author

Choose a reason for hiding this comment

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

I created this small component because the of avoiding the code duplication as it's used twice within this file.

<Stack inline grow={false} justify="end">
{actions}
</Stack>
);

export default function CardSection({
title,
Expand Down Expand Up @@ -68,24 +75,35 @@ export default function CardSection({
onKeyDown={onClick == null ? undefined : handleKeyDown(onClick)}
>
{(title != null || header != null) && expandable && (
<button
type="button"
className="p-400 lm:p-600 hover:bg-white-normal-hover w-full"
aria-expanded={opened}
aria-controls={slideID}
onClick={handleClick}
<div
className={cx(
"hover:bg-white-normal-hover p-400 lm:p-600 gap-300 relative z-10 flex cursor-pointer items-center",
"has-[.orbit-card-header-button:focus]:focus-within:rounded-100 has-[.orbit-card-header-button:focus]:focus-within:outline-blue-normal has-[.orbit-card-header-button:focus]:focus-within:outline has-[.orbit-card-header-button:focus]:focus-within:outline-2",
Copy link
Member Author

Choose a reason for hiding this comment

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

As there is not defined focus state for this specific element, I set 2px solid blue outline value, as it is used for example in InputGroup component.

)}
>
<Header
title={title}
titleAs={titleAs}
description={description}
expandable={expandable}
header={header}
expanded={opened}
actions={actions}
isSection
/>
</button>
<button
type="button"
className={cx(
"orbit-card-header-button w-full focus:outline-none",
"before:absolute before:inset-0",
)}
aria-expanded={opened}
aria-controls={slideID}
onClick={handleClick}
>
<Header
title={title}
titleAs={titleAs}
description={description}
expandable={expandable}
header={header}
expanded={opened}
actions={Boolean(actions)}
Copy link
Member Author

Choose a reason for hiding this comment

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

On this line, I convert actions to a boolean value. The reason is that Header component renders ArrowButton component in case when actions === null. I want to avoid having this component present as actions are rendered below (if available). ⬇️

isSection
/>
</button>
{actions && <Actions actions={actions} />}
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to render actions here as well as we want separate actions from Header component in case we're using this CardSection component.

</div>
)}

{(title != null || header != null) && !expandable && (
Expand All @@ -100,6 +118,7 @@ export default function CardSection({
actions={actions}
isSection
/>
{actions && <Actions actions={actions} />}
</div>
)}

Expand Down
2 changes: 1 addition & 1 deletion packages/orbit-components/src/Card/components/Header.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ const Header = ({
color="secondary"
/>
)}
{actions && (
{React.isValidElement(actions) && (
Copy link
Member Author

Choose a reason for hiding this comment

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

I've added this React.isValisElement function. In case we have actions in CardSelection component, then rendering actions here is redundant. In case actions is empty, we don't want to render <Stack> element below at all.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Feels a bit hacky, but it works for now

<Stack inline grow={false} justify="end">
{actions}
</Stack>
Expand Down
Loading