Skip to content

Conversation

@ze-flo
Copy link
Contributor

@ze-flo ze-flo commented May 13, 2025

Description

This PR adds fallbackPlacements support to both Tooltip and TooltipModal, aligning their behavior with MenuList.

Details

  • Adds support for the fallbackPlacements prop in Tooltip and TooltipModal.
  • Allows users to specify alternative positions for the overlay when the default placement is not viable due to space constraints.
  • Updates relevant Storybook stories.
Screen.Recording.2025-05-13.at.6.58.27.AM.mov

Checklist

  • [ ] 👌 design updates will be Garden Designer approved (add the designer as a reviewer)
  • 🌐 demo is up-to-date (npm start)
  • ⬅️ renders as expected with reversed (RTL) direction
  • [ ] ⚫ renders as expected in dark mode
  • [ ] 🤘 renders as expected with Bedrock CSS (?bedrock)
  • [ ] 💂‍♂️ includes new unit tests. Maintain existing coverage (always >= 96%)
  • ♿ tested for WCAG 2.1 AA accessibility compliance
  • 📝 tested in Chrome, Firefox, Safari, and Edge

@coveralls
Copy link

Coverage Status

coverage: 95.504% (+0.001%) from 95.503%
when pulling dcaee9b on ze-flo/fallback_placements
into 0101f3c on main.

</TooltipDialog>
<Grid>
<Grid.Row style={{ height: 'calc(100vh - 80px)' }}>
<Grid.Row style={{ height: 'calc(100vh - 200px)' }}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Offset story content to allow for overlay flip.

});

const [floatingPlacement] = getFloatingPlacements(
const [floatingPlacement, fallbackPlacements] = getFloatingPlacements(
Copy link
Contributor Author

@ze-flo ze-flo May 13, 2025

Choose a reason for hiding this comment

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

if props.fallbackPlacements is undefined, getFloatingPlacements will generate opposite fallbackPlacements.

const toFallbackPlacements = (
primaryPlacement: FloatingPlacement,
theme: IGardenTheme,
fallbackPlacements?: Placement[]
): FloatingPlacement[] => {
if (Array.isArray(fallbackPlacements) && fallbackPlacements.length > 0) {
return fallbackPlacements.map(fallbackPlacement =>
toFloatingPlacement(fallbackPlacement, theme)
);
}
const side = primaryPlacement.split('-')[0];
const sameSideFallbackPlacements = [...SIDE_FALLBACKS_MAP[side]];
const oppositeSideFallbackPlacements = SIDE_FALLBACKS_MAP[SIDE_OPPOSITE_MAP[side]];
// Remove the primary placement from the list of same-side fallbacks to
// prevent extra work for Floating-UI
sameSideFallbackPlacements.splice(sameSideFallbackPlacements.indexOf(primaryPlacement), 1);
return [...sameSideFallbackPlacements, ...oppositeSideFallbackPlacements];
};

Should we set a sensible default (e.g.: ['bottom']) to both Tooltip and TooltipDialog?

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't due to the fact that floating-ui already attempts to position within available space

appendToNode: PropTypes.any,
referenceElement: PropTypes.any,
placement: PropTypes.any,
// @ts-expect-error Validation error due to incorrect type inference when component is wrapped in forwardRef
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct type inference for RFCs.
Screenshot 2025-05-13 at 7 12 48 AM

Incorrect type inference for components wrapped in forwardRef.
Screenshot 2025-05-13 at 7 13 28 AM

🤔

Copy link
Member

Choose a reason for hiding this comment

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

Probably why George went with fallbackPlacements: PropTypes.arrayOf(PropTypes.any) on Menu.

@ze-flo ze-flo marked this pull request as ready for review May 13, 2025 17:16
@ze-flo ze-flo requested a review from a team as a code owner May 13, 2025 17:16
});

const [floatingPlacement] = getFloatingPlacements(
const [floatingPlacement, fallbackPlacements] = getFloatingPlacements(
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't due to the fact that floating-ui already attempts to position within available space

appendToNode: PropTypes.any,
referenceElement: PropTypes.any,
placement: PropTypes.any,
// @ts-expect-error Validation error due to incorrect type inference when component is wrapped in forwardRef
Copy link
Member

Choose a reason for hiding this comment

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

Probably why George went with fallbackPlacements: PropTypes.arrayOf(PropTypes.any) on Menu.

@ze-flo ze-flo merged commit e16818a into main May 14, 2025
9 checks passed
@ze-flo ze-flo deleted the ze-flo/fallback_placements branch May 14, 2025 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants