Skip to content

Commit

Permalink
fix(popup): Remove containerElement prop from Popper (#1524)
Browse files Browse the repository at this point in the history
* fix(popup): Remove containerElement prop from Popper

* chore!: Combine Icon Buttons with Primary, Secondary and Tertiary (#1477)

Fixes: #1379

[category:Components]

-  Combined Icon Buttons with Primary, Secondary, and Tertiary buttons
- Remove IconButton component
-  Add a new XS, L sizes
-  Removed the `toggled` prop when migrating over Icon Buttons
- Converted `SegmentedControl` into a compound component and it no longer renders `IconButton` as children
- Changed the values of `IconPosition`: `left` | `right` - > `start` | `end`
- Refactored `AccentIcon`, `AppletIcon`, `Graphic`, `Icon`, `Svg`, `SystemIcon`, and `SystemIconCircle` to use create component and remove `iconRef` prop and now just pass the ref forward
- Remove `dataLabel` prop from `PrimaryButton` and `SecondaryButton`

* feat(popup): Add codemod to remove containerElement prop from Popper

Co-authored-by: Manuel Carrera <[email protected]>
Co-authored-by: Alan B Smith <[email protected]>
  • Loading branch information
3 people authored Apr 7, 2022
1 parent 3b8329e commit 5f4eca7
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 12 deletions.
41 changes: 41 additions & 0 deletions modules/codemod/lib/v7/removePropFromPopper.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import {API, FileInfo, Options, JSXElement} from 'jscodeshift';
import {getImportRenameMap} from './utils/getImportRenameMap';
import {hasImportSpecifiers} from '../v6/utils';

const popupBarPackage = '@workday/canvas-kit-react/popup';

export default function transformer(file: FileInfo, api: API, options: Options) {
const j = api.jscodeshift;

const root = j(file.source);

if (!hasImportSpecifiers(api, root, popupBarPackage, ['Popper'])) {
return file.source;
}

const {importMap, styledMap} = getImportRenameMap(j, root, popupBarPackage);

root
.find(
j.JSXElement,
(value: JSXElement) =>
value.openingElement.name.type === 'JSXIdentifier' &&
(value.openingElement.name.name === importMap.Popper ||
value.openingElement.name.name === styledMap.Popper)
)
.forEach(nodePath => {
const attributes = nodePath.value.openingElement.attributes;
// Remove containerElement prop from Popper component
if (attributes) {
nodePath.value.openingElement.attributes = attributes.filter(item =>
item.type === 'JSXAttribute' &&
item.name.type === 'JSXIdentifier' &&
['containerElement'].includes(item.name.name)
? false
: true
);
}
});

return root.toSource();
}
39 changes: 39 additions & 0 deletions modules/codemod/lib/v7/spec/removePropFromPopper.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import {expectTransformFactory} from './expectTransformFactory';
import transform from '../removePropFromPopper';
import {stripIndent} from 'common-tags';

const expectTransform = expectTransformFactory(transform);

describe('removePropFromPopper', () => {
it('should remove containerElement prop from Popper component', () => {
const input = stripIndent`
import {Popper} from '@workday/canvas-kit-react/popup';
<Popper containerElement="any element here" />
`;

const expected = stripIndent`
import {Popper} from '@workday/canvas-kit-react/popup';
<Popper />
`;

expectTransform(input, expected);
});

it('should remove containerElement prop from Popper component imported from main package', () => {
const input = stripIndent`
import {Popper} from '@workday/canvas-kit-react';
<Popper containerElement="any element here" />
`;

const expected = stripIndent`
import {Popper} from '@workday/canvas-kit-react';
<Popper />
`;

expectTransform(input, expected);
});
});
8 changes: 7 additions & 1 deletion modules/docs/mdx/7.0-MIGRATION-GUIDE.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ any questions about the update.
- [Icons](#icons)
- [ActionBar Component Updates](#actionbar-component-updates)
- [Status Indicator Width](#status-indicator-width)
- [Popup Cards](popup-cards)
- [Popup Cards](#popup-cards)
- [Popper Props Update](#popper-props-update)
- [Component Promotions](#component-promotion)
- [Token Updates](#token-updates)
- [Depth Tokens](#depth-tokens)
Expand Down Expand Up @@ -466,6 +467,11 @@ If your code contains any hacks to make a `Modal` overflow, these hacks should n
`max-height` of the `Modal.Body` element using calculations. These should be removed. The
`Popup.Card` now has a max height and the `Popup.Body` height is automatically calculated.

## Popper Props Update

We removed the `containerElement` prop from Popper component because it's no longer needed with
Fullscreen API.

## Component Promotions

After some [assessment](https://github.com/Workday/canvas-kit/issues/1395) we've decided to promote
Expand Down
13 changes: 2 additions & 11 deletions modules/react/popup/lib/Popper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,6 @@ export interface PopperProps {
* `anchorElement`.
*/
children: ((props: {placement: Placement}) => React.ReactNode) | React.ReactNode;
/**
* The element that contains the portal children when `portal` is true. It is best to not define
* this unless you know what you're doing. Popper works with a PopupStack and in order for
* z-indexes to work correctly, all Popups on your page should live on the same root element
* otherwise you risk running into rendering issues:
* https://philipwalton.com/articles/what-no-one-told-you-about-z-index/
*/
containerElement?: Element | null;
/**
* When provided, this optional callback will be used to determine positioning for the Popper element
* instead of calling `getBoundingClientRect` on the `anchorElement` prop. Use this when you need
Expand Down Expand Up @@ -62,7 +54,7 @@ export interface PopperProps {
*/
popperOptions?: Partial<PopperOptions>;
/**
* If true, attach the Popper to the `containerElement`. If false, render the Popper within the
* If false, render the Popper within the
* DOM hierarchy of its parent. A non-portal Popper will constrained by the parent container
* overflows. If you set this to `false`, you may experience issues where you content gets cut off
* by scrollbars or `overflow: hidden`
Expand Down Expand Up @@ -113,7 +105,6 @@ const OpenPopper = React.forwardRef<HTMLDivElement, PopperProps>(
onPlacementChange,
children,
portal,
containerElement,
popperInstanceRef,
}: PopperProps,
ref
Expand Down Expand Up @@ -185,7 +176,7 @@ const OpenPopper = React.forwardRef<HTMLDivElement, PopperProps>(
return contents;
}

return ReactDOM.createPortal(contents, containerElement || stackRef.current!);
return ReactDOM.createPortal(contents, stackRef.current!);
}
);

Expand Down

0 comments on commit 5f4eca7

Please sign in to comment.