[refactor]: Modernize FlipCard Architecture#1547
Conversation
Signed-off-by: DANIEL KATOTO <katotodan@gmail.com>
BeforeScreen.Recording.2026-05-11.at.17.21.11.movAs you can see from the video above, I was not able to change the background of the card from the project I was working on. Setting also the height was diffucult, same as padding. AfterScreen.Recording.2026-05-14.at.00.12.36.mov |
There was a problem hiding this comment.
Code Review
This pull request refactors the FlipCard component by replacing the tuple-based children prop with explicit frontElement and backElement props and introducing a flipAction prop to support both click and hover triggers. The review feedback identifies critical accessibility issues in the click trigger, such as missing ARIA roles and keyboard event handlers, and a logic error where the hover trigger fails to respect the disableFlip prop. Additionally, the reviewer pointed out that using React.isValidElement for content rendering incorrectly excludes valid nodes like strings or numbers and that hardcoded fallback strings should be removed to support internationalization.
| const triggerProps = flipAction === 'click' | ||
| ? { onClick: handleFlip } | ||
| : { onMouseEnter: () => setFlipped(true), onMouseLeave: () => setFlipped(false) }; |
There was a problem hiding this comment.
The triggerProps logic has two issues:
- Accessibility: When
flipActionis'click', the component acts as a button but lacks arole,tabIndex, and keyboard event handlers, making it inaccessible to keyboard and screen reader users. - Correctness: The hover triggers do not check the
disableFlipprop, allowing the card to flip even when flipping is disabled.
const triggerProps = flipAction === 'click'
? {
onClick: handleFlip,
onKeyDown: (e: React.KeyboardEvent) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
handleFlip();
}
},
role: 'button',
tabIndex: 0,
'aria-pressed': flipped
}
: {
onMouseEnter: () => !disableFlip && setFlipped(true),
onMouseLeave: () => !disableFlip && setFlipped(false)
};
| ) : ( | ||
| <BackContent>{React.isValidElement(Back) ? Back : null}</BackContent> | ||
| )} | ||
| <FrontContent>{React.isValidElement(frontElement) ? frontElement : <div>Invalid Front Content</div>}</FrontContent> |
There was a problem hiding this comment.
React.isValidElement returns false for valid React nodes such as strings or numbers. This causes the component to incorrectly display the fallback message when plain text is passed to frontElement. Additionally, hardcoding UI strings like "Invalid Front Content" violates internationalization best practices.
| <FrontContent>{React.isValidElement(frontElement) ? frontElement : <div>Invalid Front Content</div>}</FrontContent> | |
| <FrontContent>{frontElement}</FrontContent> |
References
- Avoid hardcoding UI strings (such as button labels) in shared components. Expose these strings as configurable props to support internationalization (i18n) and localization.
| )} | ||
| <FrontContent>{React.isValidElement(frontElement) ? frontElement : <div>Invalid Front Content</div>}</FrontContent> | ||
|
|
||
| <BackContent>{React.isValidElement(backElement) ? backElement : <div>Invalid Back Content</div>}</BackContent> |
There was a problem hiding this comment.
Similar to the front face, using React.isValidElement prevents valid non-element nodes (like strings) from being rendered correctly on the back face. It is recommended to render the prop directly.
| <BackContent>{React.isValidElement(backElement) ? backElement : <div>Invalid Back Content</div>}</BackContent> | |
| <BackContent>{backElement}</BackContent> |
References
- Avoid hardcoding UI strings (such as button labels) in shared components. Expose these strings as configurable props to support internationalization (i18n) and localization.
Signed-off-by: DANIEL KATOTO <katotodan@gmail.com>
Notes for Reviewers
Description
This PR refactors the FlipCard component to align with modern CSS 3D animation standards and improves the Developer Experience (DX).
Changes
Logic: Switched from a JS-timed visibility toggle to a CSS 3D transform stack. The front and back faces are now rendered simultaneously but positioned back-to-back.
Previously the background, and other properties were related to the theme (ThemeProvider); theme can be set differently based on a project. This PR aims to give a developer much control over the background, padding, size of the component from the project this component is imported to.
API:
Deprecated array-based children.
Introduced frontElement and backElement for better clarity.
Added flipAction (hover/click) to increase component versatility.
Testing Instructions
Import FlipCard and pass two distinct components to frontElement and backElement.
Toggle the flipAction prop between hover and click (set to click by default).
Verify that the animation remains smooth even when the CPU is under load (since it is now GPU-accelerated).
Ensure that the container have a reasonable size cause the FlipCard will take the entire size of its container.
This PR fixes #1546
Signed commits