Skip to content

Commit

Permalink
fix(popover): Deprecate PopoverAlignment type values (#16346)
Browse files Browse the repository at this point in the history
* fix(popover wip): custom fn to warn,deprecate old values,and unite both

* fix(popover): align prop values updated with new ones in repro

* chore(popover): type fixes

* chore(popover): message string updated

* refactor(16467): deprecateValuesWithin signature changed
  • Loading branch information
2nikhiltom authored May 15, 2024
1 parent 9788258 commit b62c6b6
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 70 deletions.
28 changes: 1 addition & 27 deletions packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -6062,33 +6062,7 @@ Map {
"Popover" => Object {
"$$typeof": Symbol(react.forward_ref),
"propTypes": Object {
"align": Object {
"args": Array [
Array [
"top",
"top-left",
"top-right",
"bottom",
"bottom-left",
"bottom-right",
"left",
"left-bottom",
"left-top",
"right",
"right-bottom",
"right-top",
"top-start",
"top-end",
"bottom-start",
"bottom-end",
"left-end",
"left-start",
"right-end",
"right-start",
],
],
"type": "oneOf",
},
"align": [Function],
"as": Object {
"args": Array [
Array [
Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/components/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,10 @@ const Button = React.forwardRef(function Button<T extends React.ElementType>(
align = tooltipPosition;
}
if (tooltipAlignment === 'end') {
align = `${tooltipPosition}-right`;
align = `${tooltipPosition}-end`;
}
if (tooltipAlignment === 'start') {
align = `${tooltipPosition}-left`;
align = `${tooltipPosition}-start`;
}
}

Expand Down
118 changes: 80 additions & 38 deletions packages/react/src/components/Popover/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import cx from 'classnames';
import PropTypes from 'prop-types';
import deprecateValuesWithin from '../../prop-types/deprecateValuesWithin';
import React, {
useRef,
useMemo,
Expand Down Expand Up @@ -45,20 +46,25 @@ const PopoverContext = React.createContext<PopoverContext>({
autoAlign: null,
});

export type PopoverAlignment =
/**
* Deprecated popover alignment values.
* @deprecated Use NewPopoverAlignment instead.
*/
export type DeprecatedPopoverAlignment =
| 'top-left'
| 'top-right'
| 'bottom-left'
| 'bottom-right'
| 'left-bottom'
| 'left-top'
| 'right-bottom'
| 'right-top';

export type NewPopoverAlignment =
| 'top'
| 'top-left' // deprecated
| 'top-right' // deprecated
| 'bottom'
| 'bottom-left' // deprecated
| 'bottom-right' // deprecated
| 'left'
| 'left-bottom' // deprecated
| 'left-top' // deprecated
| 'right'
| 'right-bottom' // deprecated
| 'right-top' // deprecated
// new values to match floating-ui
| 'top-start'
| 'top-end'
| 'bottom-start'
Expand All @@ -68,9 +74,24 @@ export type PopoverAlignment =
| 'right-end'
| 'right-start';

export type PopoverAlignment = DeprecatedPopoverAlignment | NewPopoverAlignment;

const propMappingFunction = (deprecatedValue) => {
const mapping = {
'top-left': 'top-start',
'top-right': 'top-end',
'bottom-left': 'bottom-start',
'bottom-right': 'bottom-end',
'left-bottom': 'left-end',
'left-top': 'left-start',
'right-bottom': 'right-end',
'right-top': 'right-start',
};
return mapping[deprecatedValue];
};
interface PopoverBaseProps {
/**
* Specify how the popover should align with the trigger element
* Specify how the popover should align with the trigger element.
*/
align?: PopoverAlignment;

Expand Down Expand Up @@ -157,6 +178,7 @@ export const Popover: PopoverComponent = React.forwardRef(
const floating = useRef<HTMLSpanElement>(null);
const caretRef = useRef<HTMLSpanElement>(null);
const popover = useRef<Element>(null);

let align = mapPopoverAlignProp(initialAlign);

// If the `Popover` is the last focusable item in the tab order, it should also close when the browser window loses focus (#12922)
Expand Down Expand Up @@ -402,34 +424,54 @@ if (__DEV__) {
Popover.propTypes = {
/**
* Specify how the popover should align with the trigger element
*/
align: PropTypes.oneOf([
'top',
'top-left', // deprecated use top-start instead
'top-right', // deprecated use top-end instead

'bottom',
'bottom-left', // deprecated use bottom-start instead
'bottom-right', // deprecated use bottom-end instead

'left',
'left-bottom', // deprecated use left-end instead
'left-top', // deprecated use left-start instead

'right',
'right-bottom', // deprecated use right-end instead
'right-top', // deprecated use right-start instead

// new values to match floating-ui
'top-start',
'top-end',
'bottom-start',
'bottom-end',
'left-end',
'left-start',
'right-end',
'right-start',
]),
align: deprecateValuesWithin(
PropTypes.oneOf([
'top',
'top-left', // deprecated use top-start instead
'top-right', // deprecated use top-end instead

'bottom',
'bottom-left', // deprecated use bottom-start instead
'bottom-right', // deprecated use bottom-end instead

'left',
'left-bottom', // deprecated use left-end instead
'left-top', // deprecated use left-start instead

'right',
'right-bottom', // deprecated use right-end instead
'right-top', // deprecated use right-start instead

// new values to match floating-ui
'top-start',
'top-end',
'bottom-start',
'bottom-end',
'left-end',
'left-start',
'right-end',
'right-start',
]),
//allowed prop values
[
'top',
'top-start',
'top-end',
'bottom',
'bottom-start',
'bottom-end',
'left',
'left-start',
'left-end',
'right',
'right-start',
'right-end',
],
//optional mapper function
propMappingFunction
),

/**
* Provide a custom element or component to render the top-level node for the
Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/components/TextInput/PasswordInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -318,10 +318,10 @@ const PasswordInput = React.forwardRef(function PasswordInput(
align = tooltipPosition;
}
if (tooltipAlignment === 'end') {
align = `${tooltipPosition}-right`;
align = `${tooltipPosition}-end`;
}
if (tooltipAlignment === 'start') {
align = `${tooltipPosition}-left`;
align = `${tooltipPosition}-start`;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export interface DefinitionTooltipProps
}

const DefinitionTooltip: React.FC<DefinitionTooltipProps> = ({
align = 'bottom-left',
align = 'bottom-start',
className,
children,
definition,
Expand Down
48 changes: 48 additions & 0 deletions packages/react/src/prop-types/deprecateValuesWithin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* 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.
*/

import { warning } from '../internal/warning';

const didWarnAboutDeprecation = {};

export default function deprecateValuesWithin(
propType,
allowedValues,
propMappingFunction
) {
return function checker(props, propName, componentName, ...rest) {
if (props[propName] === undefined) {
return;
}

if (
!didWarnAboutDeprecation[componentName] ||
!didWarnAboutDeprecation[componentName][propName]
) {
didWarnAboutDeprecation[componentName] = {
...didWarnAboutDeprecation[componentName],
[propName]: true,
};

const deprecatedValue = props[propName];
const newValue = propMappingFunction
? propMappingFunction(deprecatedValue)
: null;

if (allowedValues && !allowedValues.includes(deprecatedValue)) {
const message = propMappingFunction
? `"${deprecatedValue}" is a deprecated value for the "${propName}" prop on the "${componentName}" component. Use "${newValue}" instead. "${deprecatedValue}" will be removed in the next major release.`
: `"${deprecatedValue}" is a deprecated value for the "${propName}" prop on the "${componentName}" component. Allowed values is/are: ${allowedValues.join(
', '
)}. "${deprecatedValue}" will be removed in the next major release. `;

warning(false, message);
}
}
return propType(props, propName, componentName, ...rest);
};
}

0 comments on commit b62c6b6

Please sign in to comment.