From 7b57f2e3a814ae719b6fd511a6ac08b13a8fcf64 Mon Sep 17 00:00:00 2001 From: Stephen Lee Date: Mon, 16 Sep 2024 15:35:22 -0700 Subject: [PATCH] LG-4516: deprecate justify="fit" from popover and related components (#2474) * Remove justify fit from @leafygreen-ui/popover * Remove justify fit usage in packages using Popover * Changeset --- .changeset/pink-worms-tap.md | 12 +++++++++++ packages/info-sprinkle/README.md | 2 +- packages/inline-definition/README.md | 2 +- packages/menu/src/Menu.stories.tsx | 4 ---- packages/popover/README.md | 2 +- packages/popover/src/Popover.testutils.tsx | 2 -- packages/popover/src/Popover.types.ts | 2 -- packages/popover/src/Popover/Popover.tsx | 2 +- packages/popover/src/utils/positionUtils.ts | 4 ---- packages/tooltip/README.md | 2 +- packages/tooltip/src/Tooltip.stories.tsx | 19 ----------------- packages/tooltip/src/Tooltip/Tooltip.tsx | 14 +++++-------- packages/tooltip/src/Tooltip/tooltipUtils.tsx | 21 ------------------- 13 files changed, 22 insertions(+), 66 deletions(-) create mode 100644 .changeset/pink-worms-tap.md diff --git a/.changeset/pink-worms-tap.md b/.changeset/pink-worms-tap.md new file mode 100644 index 0000000000..a8467e05c7 --- /dev/null +++ b/.changeset/pink-worms-tap.md @@ -0,0 +1,12 @@ +--- +'@leafygreen-ui/popover': major +'@leafygreen-ui/inline-definition': major +'@leafygreen-ui/info-sprinkle': major +'@leafygreen-ui/date-picker': major +'@leafygreen-ui/copyable': major +'@leafygreen-ui/tooltip': major +'@leafygreen-ui/code': major +'@leafygreen-ui/menu': major +--- + +[LG-4516](https://jira.mongodb.org/browse/LG-4516): Deprecates justify="fit"; use justify="middle" instead diff --git a/packages/info-sprinkle/README.md b/packages/info-sprinkle/README.md index 51b67224ff..070282d167 100644 --- a/packages/info-sprinkle/README.md +++ b/packages/info-sprinkle/README.md @@ -42,7 +42,7 @@ import { InfoSprinkle } from `@leafygreen-ui/info-sprinkle`; | `setOpen` | `function` | If controlling the component, pass state handling function to setOpen prop. This will keep the consuming application's state in-sync with LeafyGreen's state, while the `` component responds to events such as backdrop clicks and a user pressing the Escape key. | `(boolean) => boolean` | | `shouldClose` | `function` | Callback that should return a boolean that determines whether or not the `` should close when a user tries to close it. | `() => true` | | `align` | `'top'`, `'bottom'`, `'left'`, `'right'` | Determines the preferred alignment of the `` component relative to the element passed to the `trigger` prop. | `'top'` | -| `justify` | `'start'`, `'middle'`, `'end'`, `'fit'` | Determines the preferred justification of the `` component (based on the alignment) relative to the element passed to the `trigger` prop. | `'start'` | +| `justify` | `'start'`, `'middle'`, `'end'` | Determines the preferred justification of the `` component (based on the alignment) relative to the element passed to the `trigger` prop. | `'start'` | | `darkMode` | `boolean` | Determines if the `` will appear in dark mode. | `false` | | `id` | `string` | `id` applied to `` component | | | `className` | `string` | Applies a className to Tooltip container | | diff --git a/packages/inline-definition/README.md b/packages/inline-definition/README.md index 04bfd4419b..eb2f3b014b 100644 --- a/packages/inline-definition/README.md +++ b/packages/inline-definition/README.md @@ -58,6 +58,6 @@ npm install @leafygreen-ui/inline-definition | `children` | `string` | Text that will appear underlined | | | `className` | `string` | className will be applied to the trigger element | | | `align` | `'top'`, `'bottom'`, `'left'`, `'right'` | Determines the preferred alignment of the tooltip relative to the component's children. | `'top'` | -| `justify` | `'start'`, `'middle'`, `'end'`, `'fit'` | Determines the preferred justification of the tooltip (based on the alignment) relative to the element passed to the component's children. | `'start'` | +| `justify` | `'start'`, `'middle'`, `'end'` | Determines the preferred justification of the tooltip (based on the alignment) relative to the element passed to the component's children. | `'start'` | | `darkMode` | `boolean` | Determines if the component will appear in dark mode. | `false` | | `tooltipClassName` | `string` | className to be applied to the tooltip element | | diff --git a/packages/menu/src/Menu.stories.tsx b/packages/menu/src/Menu.stories.tsx index 67614b59ec..792a4916b5 100644 --- a/packages/menu/src/Menu.stories.tsx +++ b/packages/menu/src/Menu.stories.tsx @@ -247,10 +247,6 @@ export const Generated = { { align: [Align.CenterHorizontal, Align.CenterVertical], }, - { - justify: Justify.Fit, - align: [Align.Left, Align.Right], - }, ], decorator: (Instance, ctx) => ( diff --git a/packages/popover/README.md b/packages/popover/README.md index ac624c7d19..f6f93c2dda 100644 --- a/packages/popover/README.md +++ b/packages/popover/README.md @@ -63,7 +63,7 @@ The popover component will be automatically positioned relative to its nearest p | ------------------ | ---------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------- | | `active` | `boolean` | Determines whether the Popover is active or inactive | `false` | | `align` | `'top'` \| `'bottom'` \| `'left'` \| `'right'` \| `'center-horizontal'` \| `'center-vertical'` | A string that determines the alignment of the popover relative to the `refEl`. | `'bottom'` | -| `justify` | `'start'` \| `'middle'` \| `'end'` \| `'fit'` | A string that determines the justification of the popover relative to the `refEl`. Justification will be defined relative to the `align` prop | `'start'` | +| `justify` | `'start'` \| `'middle'` \| `'end'` | A string that determines the justification of the popover relative to the `refEl`. Justification will be defined relative to the `align` prop | `'start'` | | `children` | `node` | Content that will appear inside of the `` component | | | `spacing` | `number` | Specifies the amount of spacing (in pixels) between the trigger element and the content element. | `4` | | `className` | `string` | Classname to apply to popover-content container | | diff --git a/packages/popover/src/Popover.testutils.tsx b/packages/popover/src/Popover.testutils.tsx index e179dbf778..65595e38b3 100644 --- a/packages/popover/src/Popover.testutils.tsx +++ b/packages/popover/src/Popover.testutils.tsx @@ -17,7 +17,6 @@ export const getJustify = (a: Align, j: Justify): string => { default: switch (j) { case 'middle': - case 'fit': return 'center'; default: @@ -38,7 +37,6 @@ export const getAlign = (a: Align, j: Justify) => { default: switch (j) { case 'middle': - case 'fit': return 'center'; default: diff --git a/packages/popover/src/Popover.types.ts b/packages/popover/src/Popover.types.ts index 44a53a27e7..3db17dd9a8 100644 --- a/packages/popover/src/Popover.types.ts +++ b/packages/popover/src/Popover.types.ts @@ -38,13 +38,11 @@ export { Align }; * @param Start will justify content against the start of other element * @param Middle will justify content against the middle of other element * @param End will justify content against the end of other element - * @param Fit will justify content against both the start and the end of the other element */ const Justify = { Start: 'start', Middle: 'middle', End: 'end', - Fit: 'fit', } as const; type Justify = (typeof Justify)[keyof typeof Justify]; diff --git a/packages/popover/src/Popover/Popover.tsx b/packages/popover/src/Popover/Popover.tsx index 8f04658cc3..abc8874379 100644 --- a/packages/popover/src/Popover/Popover.tsx +++ b/packages/popover/src/Popover/Popover.tsx @@ -41,7 +41,7 @@ import { * @param props.active Boolean to describe whether or not Popover is active. * @param props.spacing The spacing (in pixels) between the reference element, and the popover. * @param props.align Alignment of Popover component relative to another element: `top`, `bottom`, `left`, `right`, `center-horizontal`, `center-vertical`. - * @param props.justify Justification of Popover component relative to another element: `start`, `middle`, `end`, `fit`. + * @param props.justify Justification of Popover component relative to another element: `start`, `middle`, `end`. * @param props.adjustOnMutation Should the Popover auto adjust its content when the DOM changes (using MutationObserver). * @param props.children Content to appear inside of Popover container. * @param props.className Classname applied to Popover container. diff --git a/packages/popover/src/utils/positionUtils.ts b/packages/popover/src/utils/positionUtils.ts index 14ca8b267a..7aaa4b6efd 100644 --- a/packages/popover/src/utils/positionUtils.ts +++ b/packages/popover/src/utils/positionUtils.ts @@ -115,10 +115,6 @@ export const getFloatingPlacement = ( align = Align.Bottom; } - if (justify === Justify.Fit) { - justify = Justify.Middle; - } - return justify === Justify.Middle ? align : `${align}-${justify}`; }; diff --git a/packages/tooltip/README.md b/packages/tooltip/README.md index 447d27aaa8..772b6c4d02 100644 --- a/packages/tooltip/README.md +++ b/packages/tooltip/README.md @@ -62,7 +62,7 @@ import Tooltip from '@leafygreen-ui/tooltip'; | `initialOpen` | `boolean` | Passes an initial "open" value to an uncontrolled Tooltip. | `false` | | `shouldClose` | `function` | Callback that should return a boolean that determines whether or not the `` should close when a user tries to close it. | `() => true` | | `align` | `'top'`, `'bottom'`, `'left'`, `'right'` | Determines the preferred alignment of the `` component relative to the element passed to the `trigger` prop. If no `trigger` is passed, the Tooltip will be positioned against its nearest parent element. | `'top'` | -| `justify` | `'start'`, `'middle'`, `'end'`, `'fit'` | Determines the preferred justification of the `` component (based on the alignment) relative to the element passed to the `trigger` prop. If no `trigger` is passed, the Tooltip will be positioned against its nearest parent element. | `'start'` | +| `justify` | `'start'`, `'middle'`, `'end'` | Determines the preferred justification of the `` component (based on the alignment) relative to the element passed to the `trigger` prop. If no `trigger` is passed, the Tooltip will be positioned against its nearest parent element. | `'start'` | | `trigger` | `function`, `React.ReactNode` | A `React.ReactNode` against which the `` will be positioned, and what will be used to trigger the opening and closing of the `Tooltip` component, when the `Tooltip` is uncontrolled. If no `trigger` is passed, the `Tooltip` will be positioned against its nearest parent element. If using a `ReactNode` or inline function, trigger signature is: ({children, ...rest}) => (). When using a function, you must pass `children` as an argument in order for the tooltip to render. | | | `triggerEvent` | `'hover'`, `'click'` | DOM event that triggers opening/closing of `` component | `'hover'` | | `darkMode` | `boolean` | Determines if the `` will appear in dark mode. | `false` | diff --git a/packages/tooltip/src/Tooltip.stories.tsx b/packages/tooltip/src/Tooltip.stories.tsx index fb0278fba5..8ed1b3f637 100644 --- a/packages/tooltip/src/Tooltip.stories.tsx +++ b/packages/tooltip/src/Tooltip.stories.tsx @@ -41,16 +41,6 @@ const meta: StoryMetaType = { justify: Object.values(Justify), baseFontSize: Object.values(BaseFontSize), }, - excludeCombinations: [ - { - justify: Justify.Fit, - children: longText, - }, - { - justify: Justify.Fit, - align: [Align.Left, Align.Right], - }, - ], args: { open: true, }, @@ -323,15 +313,6 @@ ShortString.args = { children: 'I am a tooltip!' }; export const LongString: StoryFn = () => <>; LongString.args = { children: longText }; -LongString.parameters = { - generate: { - excludeCombinations: [ - { - justify: Justify.Fit, - }, - ], - }, -}; export const JSXChildren: StoryFn = () => <>; JSXChildren.args = { diff --git a/packages/tooltip/src/Tooltip/Tooltip.tsx b/packages/tooltip/src/Tooltip/Tooltip.tsx index 949c3652ce..fc38d73903 100644 --- a/packages/tooltip/src/Tooltip/Tooltip.tsx +++ b/packages/tooltip/src/Tooltip/Tooltip.tsx @@ -215,18 +215,14 @@ function Tooltip({ justify={justify} adjustOnMutation={true} onClick={stopClickPropagation} - className={cx(transitionDelay, { - [css` + className={cx( + transitionDelay, + css` // Try to fit all the content on one line (until it hits max-width) // Overrides default behavior, which is to set width to size of the trigger. - // Except when justify is set to fit because the width should be the size of the trigger. - // Another exception is when justify is set to fit and the alignment is either left or right. In this case only the height should be the size of the trigger so we still want the width to fit the max content. width: max-content; - `]: - justify !== Justify.Fit || - (justify === Justify.Fit && - (align === Align.Left || align === Align.Right)), - })} + `, + )} {...popoverProps} > {({ align, justify, referenceElPos }: PopoverFunctionParameters) => { diff --git a/packages/tooltip/src/Tooltip/tooltipUtils.tsx b/packages/tooltip/src/Tooltip/tooltipUtils.tsx index 7dd64ad709..0187dc4c6b 100644 --- a/packages/tooltip/src/Tooltip/tooltipUtils.tsx +++ b/packages/tooltip/src/Tooltip/tooltipUtils.tsx @@ -112,17 +112,6 @@ export function notchPositionStyles({ break; - case Justify.Fit: - containerStyleObj.left = `${notchOffset}px`; - - if (shouldTransformPosition) { - tooltipOffsetTransform = `translateX(-${ - notchOffsetLowerBound - notchOffsetActual - }px)`; - } - - break; - case Justify.End: containerStyleObj.right = `${notchOffset}px`; @@ -178,16 +167,6 @@ export function notchPositionStyles({ containerStyleObj.bottom = '0px'; break; - case Justify.Fit: - containerStyleObj.top = `${notchOffset}px`; - - if (shouldTransformPosition) { - tooltipOffsetTransform = `translateY(-${ - notchOffsetLowerBound - notchOffsetActual - }px)`; - } - break; - case Justify.End: containerStyleObj.bottom = `${notchOffset}px`;