Skip to content

Picker - migrate prop deprecation end #3737

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.

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

Open
wants to merge 1 commit into
base: v8
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 7 additions & 12 deletions src/components/picker/PickerItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import Image from '../image';
import Text from '../text';
import {getItemLabel, isItemSelected} from './PickerPresenter';
import PickerContext from './PickerContext';
import {PickerItemProps, PickerSingleValue} from './types';
import {PickerItemProps} from './types';

/**
* @description: Picker.Item, for configuring the Picker's selectable options
Expand All @@ -29,10 +29,9 @@ const PickerItem = (props: PickerItemProps) => {
testID
} = props;
const context = useContext(PickerContext);
const {migrate} = context;
const customRenderItem = props.renderItem || context.renderItem;
// @ts-expect-error TODO: fix after removing migrate prop completely
const itemValue = !migrate && typeof value === 'object' ? value?.value : value;

const itemValue = value;
const isSelected = isItemSelected(itemValue, context.value);
const itemLabel = getItemLabel(label, value, props.getItemLabel || context.getItemLabel);
const selectedCounter = context.selectionLimit && _.isArray(context.value) && context.value?.length;
Expand Down Expand Up @@ -65,16 +64,12 @@ const PickerItem = (props: PickerItemProps) => {
const _onPress = useCallback(async (props: any) => {
// Using !(await onPress?.(item)) does not work properly when onPress is not sent
// We have to explicitly state `false` so a synchronous void (undefined) will still work as expected
if (onPress && await onPress(context.isMultiMode ? !isSelected : undefined, props) === false) {
if (onPress && (await onPress(context.isMultiMode ? !isSelected : undefined, props)) === false) {
return;
}
if (migrate) {
context.onPress(value);
} else {
// @ts-expect-error TODO: fix after removing migrate prop completely
context.onPress(typeof value === 'object' || context.isMultiMode ? value : ({value, label: itemLabel}) as PickerSingleValue);
}
}, [migrate, value, context.onPress, onPress]);
context.onPress(value);
},
[value, context.onPress, onPress]);

const onSelectedLayout = useCallback((...args: any[]) => {
_.invoke(context, 'onSelectedLayout', ...args);
Expand Down
3 changes: 1 addition & 2 deletions src/components/picker/PickerPresenter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ export function isItemSelected(childValue: PickerSingleValue, selectedValue?: Pi
if (Array.isArray(selectedValue)) {
isSelected =
_.find(selectedValue, v => {
// @ts-expect-error TODO: fix after removing migrate prop completely
return v === childValue || (typeof v === 'object' && v?.value === childValue);
return v === childValue;
}) !== undefined;
} else {
isSelected = childValue === selectedValue;
Expand Down
1 change: 0 additions & 1 deletion src/components/picker/api/picker.api.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
"https://github.com/wix/react-native-ui-lib/blob/master/demo/showcase/Picker/CustomPicker.gif?raw=true"
],
"props": [
{"name": "migrate", "type": "boolean", "description": "Temporary prop required for migration to Picker's new API"},
{"name": "value", "type": "string | number", "description": "Picker current value"},
{
"name": "onChange",
Expand Down
11 changes: 2 additions & 9 deletions src/components/picker/helpers/usePickerMigrationWarnings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,15 @@ import {LogService} from '../../../services';
import {PickerProps} from '../types';

// TODO: Remove this whole file when migration is completed
type UsePickerMigrationWarnings = Pick<
PickerProps,
'children' | 'migrate' | 'getItemLabel' | 'getItemValue' | 'onShow'
>;
type UsePickerMigrationWarnings = Pick<PickerProps, 'children' | 'getItemLabel' | 'getItemValue' | 'onShow'>;

const usePickerMigrationWarnings = (props: UsePickerMigrationWarnings) => {
const {children, migrate, getItemLabel, getItemValue, onShow} = props;
const {children, getItemLabel, getItemValue, onShow} = props;
useEffect(() => {
if (children) {
LogService.warn(`UILib Picker will stop supporting the 'children' prop in the next major version, please pass 'items' prop instead`);
}

if (migrate) {
LogService.warn(`UILib Picker will stop supporting the 'migrate' prop in the next major version, please stop using it. The picker uses the new implementation by default.`);
}

if (getItemLabel) {
LogService.warn(`UILib Picker will stop supporting the 'getItemLabel' prop in the next major version, please pass the 'getItemLabel' prop to the specific item instead`);
}
Expand Down
12 changes: 3 additions & 9 deletions src/components/picker/helpers/usePickerSelection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ import _ from 'lodash';
import {PickerProps, PickerValue, PickerSingleValue, PickerMultiValue, PickerModes} from '../types';

interface UsePickerSelectionProps
extends Pick<PickerProps, 'migrate' | 'value' | 'onChange' | 'getItemValue' | 'topBarProps' | 'mode' | 'items'> {
extends Pick<PickerProps, 'value' | 'onChange' | 'getItemValue' | 'topBarProps' | 'mode' | 'items'> {
pickerExpandableRef: RefObject<any>;
setSearchValue: (searchValue: string) => void;
}

const usePickerSelection = (props: UsePickerSelectionProps) => {
const {migrate, value, onChange, topBarProps, pickerExpandableRef, getItemValue, setSearchValue, mode, items} = props;
const {value, onChange, topBarProps, pickerExpandableRef, getItemValue, setSearchValue, mode, items} = props;
const [multiDraftValue, setMultiDraftValue] = useState(value as PickerMultiValue);
const [multiFinalValue, setMultiFinalValue] = useState(value as PickerMultiValue);

Expand All @@ -29,14 +29,8 @@ const usePickerSelection = (props: UsePickerSelectionProps) => {
[onChange]);

const toggleItemSelection = useCallback((item: PickerSingleValue) => {
let newValue;
const itemAsArray = [item];
if (!migrate) {
newValue = _.xorBy(multiDraftValue, itemAsArray, getItemValue || 'value');
} else {
newValue = _.xor(multiDraftValue, itemAsArray);
}

const newValue = _.xor(multiDraftValue, itemAsArray);
setMultiDraftValue(newValue);
},
[multiDraftValue, getItemValue]);
Expand Down
17 changes: 6 additions & 11 deletions src/components/picker/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ const Picker = React.forwardRef((props: PickerProps, ref) => {
renderItem,
children,
useSafeArea,
// TODO: Remove migrate props and migrate code
migrate = true,
accessibilityLabel,
accessibilityHint,
items: propItems,
Expand All @@ -91,7 +89,7 @@ const Picker = React.forwardRef((props: PickerProps, ref) => {
const pickerRef = useImperativePickerHandle(ref, pickerExpandable);

// TODO: Remove this when migration is completed, starting of v8
// usePickerMigrationWarnings({children, migrate, getItemLabel, getItemValue});
// usePickerMigrationWarnings({children, getItemLabel, getItemValue});

useEffect(() => {
if (propItems) {
Expand All @@ -113,7 +111,6 @@ const Picker = React.forwardRef((props: PickerProps, ref) => {
selectedCount,
toggleAllItemsSelection
} = usePickerSelection({
migrate,
value,
onChange,
pickerExpandableRef: pickerExpandable,
Expand All @@ -128,8 +125,10 @@ const Picker = React.forwardRef((props: PickerProps, ref) => {
if (propItems) {
return filteredItems.map((item: PickerItemProps) => ({
...item,
onPress: useWheelPicker && Constants.accessibility.isScreenReaderEnabled ?
() => onDoneSelecting(item.value) : undefined
onPress:
useWheelPicker && Constants.accessibility.isScreenReaderEnabled
? () => onDoneSelecting(item.value)
: undefined
}));
}
return filteredItems;
Expand Down Expand Up @@ -163,11 +162,8 @@ const Picker = React.forwardRef((props: PickerProps, ref) => {
}, []);

const contextValue = useMemo(() => {
// @ts-expect-error cleanup after removing migrate prop
const pickerValue = !migrate && typeof value === 'object' && !_.isArray(value) ? value?.value : value;
return {
migrate,
value: mode === PickerModes.MULTI ? multiDraftValue : pickerValue,
value: mode === PickerModes.MULTI ? multiDraftValue : value,
onPress: mode === PickerModes.MULTI ? toggleItemSelection : onDoneSelecting,
isMultiMode: mode === PickerModes.MULTI,
getItemValue,
Expand All @@ -180,7 +176,6 @@ const Picker = React.forwardRef((props: PickerProps, ref) => {
toggleAllItemsSelection
};
}, [
migrate,
mode,
value,
multiDraftValue,
Expand Down
7 changes: 1 addition & 6 deletions src/components/picker/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,6 @@ export interface PickerSearchStyle {
}

type PickerPropsDeprecation = {
/**
* @deprecated
* Temporary prop required for migration to Picker's new API
*/
migrate?: boolean;
/**
* @deprecated
* A function that extract the unique value out of the value prop in case value has a custom structure (e.g. {myValue, myLabel})
Expand Down Expand Up @@ -352,7 +347,7 @@ export interface PickerItemProps extends Pick<TouchableOpacityProps, 'customValu
}

export interface PickerContextProps
extends Pick<PickerProps, 'migrate' | 'value' | 'getItemValue' | 'getItemLabel' | 'renderItem' | 'selectionLimit'> {
extends Pick<PickerProps, 'value' | 'getItemValue' | 'getItemLabel' | 'renderItem' | 'selectionLimit'> {
onPress: (value: PickerSingleValue) => void;
isMultiMode: boolean;
onSelectedLayout: (event: any) => any;
Expand Down