Card: refactor component for improved accessibility #4574
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sarkaaa
changed the title
sarkaaa
changed the title
|
Storybook staging is available at https://kiwicom-orbit-sarka-card-a11y.surge.sh |
|
Size Change: +217 B (+0.05%) Total Size: 459 kB
ℹ️ View Unchanged
|
Deploying orbit with
|
| Latest commit: |
643971c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://67e3076f.orbit.pages.dev |
| Branch Preview URL: | https://sarka-card-a11y.orbit.pages.dev |
sarkaaa
force-pushed
the
sarka/card-a11y
branch
2 times, most recently
from
245a5fa to
7343393
Compare
![callstackai[bot]](https://avatars.githubusercontent.com/in/1040204?s=60&v=4){kind=small}
sarkaaa
force-pushed
the
sarka/card-a11y
branch
from
7343393 to
06448f5
Compare
sarkaaa
commented
Jan 14, 2025
| @@ -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 }) => ( | |||
There was a problem hiding this comment.
I created this small component because the of avoiding the code duplication as it's used twice within this file.
sarkaaa
commented
Jan 14, 2025
| expandable={expandable} | ||
| header={header} | ||
| expanded={opened} | ||
| actions={Boolean(actions)} |
There was a problem hiding this comment.
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). ⬇️
sarkaaa
commented
Jan 14, 2025
| @@ -68,7 +68,7 @@ const Header = ({ | |||
| color="secondary" | |||
| /> | |||
| )} | |||
| {actions && ( | |||
| {React.isValidElement(actions) && ( | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Feels a bit hacky, but it works for now
sarkaaa
commented
Jan 14, 2025
| isSection | ||
| /> | ||
| </button> | ||
| {actions && <Actions actions={actions} />} |
There was a problem hiding this comment.
Moved to render actions here as well as we want separate actions from Header component in case we're using this CardSection component.
sarkaaa
force-pushed
the
sarka/card-a11y
branch
from
06448f5 to
9aaf8b0
Compare
sarkaaa
force-pushed
the
sarka/card-a11y
branch
2 times, most recently
from
6864598 to
5f5e39a
Compare
sarkaaa
changed the title
sarkaaa
commented
Jan 14, 2025
| <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", |
There was a problem hiding this comment.
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.
DSil
approved these changes
Jan 20, 2025
| @@ -13,6 +13,15 @@ import { ELEMENT_OPTIONS } from "../Heading/consts"; | |||
|
|
|||
| import Card from "."; | |||
|
|
|||
| const CardSectionHeaderContent = () => ( | |||
There was a problem hiding this comment.
Doing this doesn't help the code snippet at all. I would rather keep it as it was
| @@ -68,7 +68,7 @@ const Header = ({ | |||
| color="secondary" | |||
| /> | |||
| )} | |||
| {actions && ( | |||
| {React.isValidElement(actions) && ( | |||
There was a problem hiding this comment.
Feels a bit hacky, but it works for now
sarkaaa
force-pushed
the
sarka/card-a11y
branch
from
5f5e39a to
3cc353d
Compare
sarkaaa
force-pushed
the
sarka/card-a11y
branch
from
3cc353d to
643971c
Compare
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I got inspired here: https://kittygiraudel.com/2022/04/02/accessible-cards/#wrapping-up
Closes https://kiwicom.atlassian.net/browse/FEPLT-2196
This Pull Request meets the following criteria:
For new components:
d.tsfiles and are exported inindex.d.tsCloses: https://kiwicom.atlassian.net/browse/FEPLT-2196
✨
Description by Callstackai
This PR refactors the Card component to improve accessibility by enhancing the structure and semantics of the CardSection and Header components.
Diagrams of code changes
sequenceDiagram participant User participant CardSection participant Header participant Actions participant Stack User->>CardSection: Clicks expandable section CardSection->>Header: Renders header with title/description Header-->>CardSection: Header rendered alt Has Actions CardSection->>Actions: Renders action buttons Actions->>Stack: Wraps actions in Stack Stack-->>Actions: Actions aligned to end Actions-->>CardSection: Actions rendered end CardSection->>User: Shows/Hides expandable contentFiles Changed