Skip to content

[refactor]: Modernize FlipCard Architecture#1547

Open
Katotodan wants to merge 2 commits into
layer5io:masterfrom
Katotodan:chore/flip-card
Open

[refactor]: Modernize FlipCard Architecture#1547
Katotodan wants to merge 2 commits into
layer5io:masterfrom
Katotodan:chore/flip-card

Conversation

@Katotodan
Copy link
Copy Markdown
Member

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

  1. 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.

  2. API:

  • Deprecated array-based children.

  • Introduced frontElement and backElement for better clarity.

  • Added flipAction (hover/click) to increase component versatility.

  1. Styling: Used shouldForwardProp in MUI's styled utility to prevent custom props from leaking to the DOM.

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

  • Yes, I signed my commits.

Signed-off-by: DANIEL KATOTO <katotodan@gmail.com>
@Katotodan
Copy link
Copy Markdown
Member Author

Before

Screen.Recording.2026-05-11.at.17.21.11.mov

As 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.
This PR aims to solve that.

After

Screen.Recording.2026-05-14.at.00.12.36.mov

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

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.

Comment thread src/custom/FlipCard/FlipCard.tsx Outdated
Comment on lines +85 to +87
const triggerProps = flipAction === 'click'
? { onClick: handleFlip }
: { onMouseEnter: () => setFlipped(true), onMouseLeave: () => setFlipped(false) };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The triggerProps logic has two issues:

  1. Accessibility: When flipAction is 'click', the component acts as a button but lacks a role, tabIndex, and keyboard event handlers, making it inaccessible to keyboard and screen reader users.
  2. Correctness: The hover triggers do not check the disableFlip prop, 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) 
      };

Comment thread src/custom/FlipCard/FlipCard.tsx Outdated
) : (
<BackContent>{React.isValidElement(Back) ? Back : null}</BackContent>
)}
<FrontContent>{React.isValidElement(frontElement) ? frontElement : <div>Invalid Front Content</div>}</FrontContent>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

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.

Suggested change
<FrontContent>{React.isValidElement(frontElement) ? frontElement : <div>Invalid Front Content</div>}</FrontContent>
<FrontContent>{frontElement}</FrontContent>
References
  1. Avoid hardcoding UI strings (such as button labels) in shared components. Expose these strings as configurable props to support internationalization (i18n) and localization.

Comment thread src/custom/FlipCard/FlipCard.tsx Outdated
)}
<FrontContent>{React.isValidElement(frontElement) ? frontElement : <div>Invalid Front Content</div>}</FrontContent>

<BackContent>{React.isValidElement(backElement) ? backElement : <div>Invalid Back Content</div>}</BackContent>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

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.

Suggested change
<BackContent>{React.isValidElement(backElement) ? backElement : <div>Invalid Back Content</div>}</BackContent>
<BackContent>{backElement}</BackContent>
References
  1. 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor FlipCard to use CSS 3D Transforms and Explicit Props

1 participant