Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

LG-4516: deprecate justify="fit" from popover and related components #2474

Merged
merged 3 commits into from
Sep 16, 2024
Merged

LG-4516: deprecate justify="fit" from popover and related components #2474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a22da18
Remove justify fit from @leafygreen-ui/popover
stephl3 Sep 16, 2024
d4123a2
Remove justify fit usage in packages using Popover
stephl3 Sep 16, 2024
15e4bd0
Changeset
stephl3 Sep 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .changeset/pink-worms-tap.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion packages/info-sprinkle/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<InfoSprinkle />` 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 `<InfoSprinkle />` should close when a user tries to close it. | `() => true` |
| `align` | `'top'`, `'bottom'`, `'left'`, `'right'` | Determines the preferred alignment of the `<InfoSprinkle />` component relative to the element passed to the `trigger` prop. | `'top'` |
| `justify` | `'start'`, `'middle'`, `'end'`, `'fit'` | Determines the preferred justification of the `<InfoSprinkle />` 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 `<InfoSprinkle />` component (based on the alignment) relative to the element passed to the `trigger` prop. | `'start'` |
| `darkMode` | `boolean` | Determines if the `<InfoSprinkle />` will appear in dark mode. | `false` |
| `id` | `string` | `id` applied to `<InfoSprinkle />` component | |
| `className` | `string` | Applies a className to Tooltip container | |
Expand Down
2 changes: 1 addition & 1 deletion packages/inline-definition/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 | |
4 changes: 0 additions & 4 deletions packages/menu/src/Menu.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,6 @@ export const Generated = {
{
align: [Align.CenterHorizontal, Align.CenterVertical],
},
{
justify: Justify.Fit,
align: [Align.Left, Align.Right],
},
],
decorator: (Instance, ctx) => (
<LeafyGreenProvider darkMode={ctx?.args?.darkMode}>
Expand Down
2 changes: 1 addition & 1 deletion packages/popover/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'` |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we intentionally removing end here as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oops no that's a mistake. added that back

| `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 `<Popover />` component | |
| `spacing` | `number` | Specifies the amount of spacing (in pixels) between the trigger element and the content element. | `10` |
| `className` | `string` | Classname to apply to popover-content container | |
Expand Down
2 changes: 0 additions & 2 deletions packages/popover/src/Popover.testutils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ export const getJustify = (a: Align, j: Justify): string => {
default:
switch (j) {
case 'middle':
case 'fit':
return 'center';

default:
Expand All @@ -38,7 +37,6 @@ export const getAlign = (a: Align, j: Justify) => {
default:
switch (j) {
case 'middle':
case 'fit':
return 'center';

default:
Expand Down
2 changes: 0 additions & 2 deletions packages/popover/src/Popover.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
2 changes: 1 addition & 1 deletion packages/popover/src/Popover/Popover.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,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.
Expand Down
4 changes: 0 additions & 4 deletions packages/popover/src/utils/positionUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,6 @@ export const getFloatingPlacement = (
align = Align.Bottom;
}

if (justify === Justify.Fit) {
justify = Justify.Middle;
}

return justify === Justify.Middle ? align : `${align}-${justify}`;
};

Expand Down
2 changes: 1 addition & 1 deletion packages/tooltip/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<Tooltip />` should close when a user tries to close it. | `() => true` |
| `align` | `'top'`, `'bottom'`, `'left'`, `'right'` | Determines the preferred alignment of the `<Tooltip />` 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 `<Tooltip />` 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 `<Tooltip />` 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 `<Tooltip />` 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}) => (<button {...rest}>trigger {children}</button>). 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 `<Tooltip />` component | `'hover'` |
| `darkMode` | `boolean` | Determines if the `<Tooltip />` will appear in dark mode. | `false` |
Expand Down
19 changes: 0 additions & 19 deletions packages/tooltip/src/Tooltip.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,6 @@ const meta: StoryMetaType<typeof Tooltip> = {
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,
},
Expand Down Expand Up @@ -323,15 +313,6 @@ ShortString.args = { children: 'I am a tooltip!' };

export const LongString: StoryFn<typeof Tooltip> = () => <></>;
LongString.args = { children: longText };
LongString.parameters = {
generate: {
excludeCombinations: [
{
justify: Justify.Fit,
},
],
},
};

export const JSXChildren: StoryFn<typeof Tooltip> = () => <></>;
JSXChildren.args = {
Expand Down
14 changes: 5 additions & 9 deletions packages/tooltip/src/Tooltip/Tooltip.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
21 changes: 0 additions & 21 deletions packages/tooltip/src/Tooltip/tooltipUtils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`;

Expand Down Expand Up @@ -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`;

Expand Down
Loading