Skip to content

Commit

Permalink
fix(menu): carbon-design-system#16882 Menu's target prop fix
Browse files Browse the repository at this point in the history
  • Loading branch information
NabeelAyubee committed Aug 12, 2024
1 parent 51d2a21 commit a105f85
Show file tree
Hide file tree
Showing 14 changed files with 229 additions and 8 deletions.
9 changes: 9 additions & 0 deletions .all-contributorsrc
Original file line number Diff line number Diff line change
Expand Up @@ -1587,6 +1587,15 @@
"contributions": [
"code"
]
},
{
"login": "NabeelAyubee",
"name": "Md Nabeel Ayubee",
"avatar_url": "https://avatars.githubusercontent.com/u/64087875?v=4",
"profile": "https://github.com/NabeelAyubee",
"contributions": [
"code"
]
}
],
"commitConvention": "none"
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ check out our [Contributing Guide](/.github/CONTRIBUTING.md) and our
<td align="center"><a href="https://github.com/lluisrojass"><img src="https://avatars.githubusercontent.com/u/15043356?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Luis</b></sub></a><br /><a href="https://github.com/carbon-design-system/carbon/commits?author=lluisrojass" title="Code">💻</a></td>
<td align="center"><a href="https://github.com/lharrison13"><img src="https://avatars.githubusercontent.com/u/172074450?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Luke Harrison</b></sub></a><br /><a href="https://github.com/carbon-design-system/carbon/commits?author=lharrison13" title="Code">💻</a></td>
<td align="center"><a href="https://github.com/tekno0ryder"><img src="https://avatars.githubusercontent.com/u/8721803?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Ahmed Alsinan</b></sub></a><br /><a href="https://github.com/carbon-design-system/carbon/commits?author=tekno0ryder" title="Code">💻</a></td>
<td align="center"><a href="https://github.com/NabeelAyubee"><img src="https://avatars.githubusercontent.com/u/64087875?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Md Nabeel Ayubee</b></sub></a><br /><a href="https://github.com/carbon-design-system/carbon/commits?author=NabeelAyubee" title="Code">💻</a></td>
</tr>
</table>

Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/components/Menu/Menu-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe('Menu', () => {
const el = document.createElement('div');
document.body.appendChild(el);
el.classList.add('custom-class');
render(<Menu open target={el} />);
render(<Menu open menuTarget={el} />);

// eslint-disable-next-line testing-library/no-node-access
expect(document.querySelector('.custom-class')).toBeInTheDocument();
Expand Down
20 changes: 18 additions & 2 deletions packages/react/src/components/Menu/Menu.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,13 @@ export const Playground = (args) => {
const selectableOnChange = action('onChange (MenuItemSelectable)');
const radioOnChange = action('onChange (MenuItemRadioGroup)');

const target = document.getElementById('storybook-root');
const menuTarget = document.getElementById('storybook-root');

return (
<Menu {...args} target={target} x={document?.dir === 'rtl' ? 250 : 0}>
<Menu
{...args}
menuTarget={menuTarget}
x={document?.dir === 'rtl' ? 250 : 0}>
<MenuItem label="Share with">
<MenuItemRadioGroup
label="Share with"
Expand Down Expand Up @@ -92,3 +95,16 @@ Playground.args = {
onClose: action('onClose'),
open: true,
};

Playground.argTypes = {
menuTarget: {
table: {
disable: true,
},
},
target: {
table: {
disable: true,
},
},
};
31 changes: 28 additions & 3 deletions packages/react/src/components/Menu/Menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ interface MenuProps extends React.HTMLAttributes<HTMLUListElement> {
/**
* Specify a DOM node where the Menu should be rendered in. Defaults to document.body.
*/
menuTarget?: Element;

/**
* Specify a DOM node where the Menu should be rendered in. Defaults to document.body.
* (Deprecated prop)
*/
target?: Element;

/**
Expand Down Expand Up @@ -118,6 +124,7 @@ const Menu = forwardRef<HTMLUListElement, MenuProps>(function Menu(
legacyAutoalign = 'true',
// eslint-disable-next-line ssr-friendly/no-dom-globals-in-react-fc
target = canUseDOM && document.body,
menuTarget = canUseDOM && document.body,
x = 0,
y = 0,
...rest
Expand All @@ -132,6 +139,13 @@ const Menu = forwardRef<HTMLUListElement, MenuProps>(function Menu(

const isRoot = context.state.isRoot;

if (target && target !== document.body) {
warning(
false,
'Warning: The `target` prop is deprecated and will be removed in a future release. Please use `menuTarget` instead.'
);
}

if (context.state.mode === 'basic' && !isRoot) {
warning(
false,
Expand Down Expand Up @@ -443,11 +457,18 @@ const Menu = forwardRef<HTMLUListElement, MenuProps>(function Menu(
</MenuContext.Provider>
);

if (!target) {
if (!(menuTarget || target)) {
return rendered;
}

return isRoot ? (open && createPortal(rendered, target)) || null : rendered;
return isRoot
? (open &&
createPortal(
rendered,
target && target !== document.body ? target : menuTarget
)) ||
null
: rendered;
});

Menu.propTypes = {
Expand Down Expand Up @@ -505,8 +526,12 @@ Menu.propTypes = {
* Specify a DOM node where the Menu should be rendered in. Defaults to document.body.
*/
// @ts-ignore-next-line -- avoid spurious (?) TS2322 error
target: PropTypes.object,
menuTarget: PropTypes.object,

/**
* Specify a DOM node where the Menu should be rendered in. Defaults to document.body.
*/
target: PropTypes.instanceOf(Element),
/**
* Specify the x position of the Menu. Either pass a single number or an array with two numbers describing your activator's boundaries ([x1, x2])
*/
Expand Down
10 changes: 10 additions & 0 deletions packages/react/src/components/MenuButton/MenuButton.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,4 +161,14 @@ Playground.argTypes = {
disable: true,
},
},
menuTarget: {
table: {
disable: true,
},
},
target: {
table: {
disable: true,
},
},
};
28 changes: 27 additions & 1 deletion packages/react/src/components/MenuButton/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,17 @@ export interface MenuButtonProps extends ComponentProps<'div'> {
* Specify the tabIndex of the button.
*/
tabIndex?: number;

/**
* Specify a DOM node where the Menu should be rendered in. Defaults to document.body.
*/
menuTarget?: Element;

/**
* Specify a DOM node where the Menu should be rendered in. Defaults to document.body.
* (Deprecated props)
*/
target?: Element;
}

const MenuButton = forwardRef<HTMLDivElement, MenuButtonProps>(
Expand All @@ -93,6 +104,8 @@ const MenuButton = forwardRef<HTMLDivElement, MenuButtonProps>(
size = 'lg',
menuAlignment = 'bottom',
tabIndex = 0,
menuTarget,
target,
...rest
},
forwardRef
Expand Down Expand Up @@ -191,7 +204,9 @@ const MenuButton = forwardRef<HTMLDivElement, MenuButtonProps>(
mode="basic"
size={size}
open={open}
onClose={handleClose}>
onClose={handleClose}
menuTarget={menuTarget}
target={target}>
{children}
</Menu>
</div>
Expand Down Expand Up @@ -250,6 +265,17 @@ MenuButton.propTypes = {
*/
// @ts-ignore-next-line -- avoid spurious (?) TS2322 error
tabIndex: PropTypes.number,

/**
* Specify a DOM node where the Menu should be rendered in. Defaults to document.body.
*/
menuTarget: PropTypes.instanceOf(Element),

/**
* Specify a DOM node where the Menu should be rendered in. Defaults to document.body.
* Deprecated props
*/
target: PropTypes.instanceOf(Element),
};

export { MenuButton };
Original file line number Diff line number Diff line change
Expand Up @@ -222,4 +222,14 @@ Playground.argTypes = {
'Specify how the menu should align with the button element `bottom-start` `bottom-end` `top-start` `top-end`',
default: 'bottom-start',
},
menuTarget: {
table: {
disable: true,
},
},
target: {
table: {
disable: true,
},
},
};
14 changes: 13 additions & 1 deletion packages/react/src/components/OverflowMenu/next/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ interface OverflowMenuProps {
| 'bottom-right'
| 'left'
| 'right';

/**
* Specify a DOM node where the Menu should be rendered in. Defaults to document.body.
*/
menuTarget?: Element;
}

const OverflowMenu = React.forwardRef<HTMLDivElement, OverflowMenuProps>(
Expand All @@ -87,6 +92,7 @@ const OverflowMenu = React.forwardRef<HTMLDivElement, OverflowMenuProps>(
size = defaultSize,
menuAlignment = 'bottom-start',
tooltipAlignment,
menuTarget,
...rest
},
forwardRef
Expand Down Expand Up @@ -205,7 +211,8 @@ const OverflowMenu = React.forwardRef<HTMLDivElement, OverflowMenuProps>(
onClose={handleClose}
x={x}
y={y}
label={label}>
label={label}
menuTarget={menuTarget}>
{children}
</Menu>
</div>
Expand Down Expand Up @@ -266,6 +273,11 @@ OverflowMenu.propTypes = {
'left',
'right',
]),

/**
* Specify a DOM node where the Menu should be rendered in. Defaults to document.body.
*/
menuTarget: PropTypes.instanceOf(Element),
};

export { OverflowMenu };
27 changes: 27 additions & 0 deletions packages/upgrade/src/upgrades.js
Original file line number Diff line number Diff line change
Expand Up @@ -312,4 +312,31 @@ export const upgrades = [
},
],
},
{
name: 'update-menu-target-prop',
description: 'Update Menu`s target prop to menuTarget',
migrate: async (options) => {
const transform = path.join(TRANSFORM_DIR, 'update-menu-target-prop.js');
const paths =
Array.isArray(options.paths) && options.paths.length > 0
? options.paths
: await glob(['**/*.js', '**/*.jsx'], {
cwd: options.workspaceDir,
ignore: [
'**/es/**',
'**/lib/**',
'**/umd/**',
'**/node_modules/**',
'**/storybook-static/**',
],
});

await run({
dry: !options.write,
transform,
paths,
verbose: options.verbose,
});
},
},
];
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
function Menu() {
return (
<div>
<Menu className="test" target={document.body} />
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
function Menu() {
return (
<div>
<Menu className="test" menuTarget={document.body} />
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* Copyright IBM Corp. 2016, 2023
*
* This source code is licensed under the Apache-2.0 license found in the
* LICENSE file in the root directory of this source tree.
*/

'use strict';

const { defineTest } = require('jscodeshift/dist/testUtils');

defineTest(__dirname, 'update-menu-target-prop');
59 changes: 59 additions & 0 deletions packages/upgrade/transforms/update-menu-target-prop.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/**
* Copyright Your Company Name. 2024
*
* This source code is licensed under the Apache-2.0 license found in the
* LICENSE file in the root directory of this source tree.
*
* Update `target` prop to `menuTarget` within <Menu> components
* and adds a deprecation warning.
*
* Transforms:
*
* <Menu target={props.target} />
*
* Into:
*
* <Menu menuTarget={props.target} />
*
* And adds the following warning at the top of the file:
*
* console.warn('The "target" prop is deprecated and will be removed in a future release. Please use "menuTarget" instead.');
*/

function transformer(file, api) {
const j = api.jscodeshift;

return (
j(file.source)
// Added deprecation warning at the top of the file
.find(j.Program)
.forEach((path) => {
path.node.body.unshift(
j.expressionStatement(
j.callExpression(
j.memberExpression(j.identifier('console'), j.identifier('warn')),
[
j.literal(
'The "target" prop is deprecated and will be removed in a future release. Please use "menuTarget" instead.'
),
]
)
)
);
})
// Finding <Menu> components and replace `target` with `menuTarget`
.find(j.JSXElement, {
openingElement: { name: { name: 'Menu' } },
})
.forEach((path) => {
path
.find(j.JSXAttribute, { name: { name: 'target' } })
.forEach((attrPath) => {
attrPath.get('name').replace(j.jsxIdentifier('menuTarget'));
});
})
.toSource({ quote: 'single' })
);
}

module.exports = transformer;

0 comments on commit a105f85

Please sign in to comment.