Skip to content

Commit

Permalink
Replace deprecated visible prop of ant-design v3 with open prop of an…
Browse files Browse the repository at this point in the history
…t-design v4 (#1855)

## Which problem is this PR solving?
- part of: #1703

## Description of the changes
- This PR replaces the deprecated visible prop of ant-design v3 with
open prop of ant-design v4
- These props are exactly the same, just the name differs.

## How was this change tested?
- unit tests, and manually

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [x] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Ansh Goyal <anshgoyal1704@gmail.com>
  • Loading branch information
anshgoyalevil committed Oct 9, 2023
1 parent b89bc46 commit 797e0d8
Show file tree
Hide file tree
Showing 11 changed files with 45 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -136,23 +136,23 @@ describe('TraceDiffHeader', () => {
expect(wrapper.state().tableVisible).toBe(null);
const popovers = wrapper.find(Popover);
expect(popovers.length).toBe(2);
popovers.forEach(popover => expect(popover.prop('visible')).toBe(false));
popovers.forEach(popover => expect(popover.prop('open')).toBe(false));

getPopoverProp(0, 'onVisibleChange')(true);
expect(getPopoverProp(0, 'visible')).toBe(true);
expect(getPopoverProp(1, 'visible')).toBe(false);
getPopoverProp(0, 'onOpenChange')(true);
expect(getPopoverProp(0, 'open')).toBe(true);
expect(getPopoverProp(1, 'open')).toBe(false);

getPopoverProp(1, 'onVisibleChange')(true);
expect(getPopoverProp(0, 'visible')).toBe(false);
expect(getPopoverProp(1, 'visible')).toBe(true);
getPopoverProp(1, 'onOpenChange')(true);
expect(getPopoverProp(0, 'open')).toBe(false);
expect(getPopoverProp(1, 'open')).toBe(true);

// repeat onVisibleChange call to test that visibility remains correct
getPopoverProp(1, 'onVisibleChange')(true);
expect(getPopoverProp(0, 'visible')).toBe(false);
expect(getPopoverProp(1, 'visible')).toBe(true);
// repeat onOpenChange call to test that visibility remains correct
getPopoverProp(1, 'onOpenChange')(true);
expect(getPopoverProp(0, 'open')).toBe(false);
expect(getPopoverProp(1, 'open')).toBe(true);

getPopoverProp(1, 'onVisibleChange')(false);
wrapper.find(Popover).forEach(popover => expect(popover.prop('visible')).toBe(false));
getPopoverProp(1, 'onOpenChange')(false);
wrapper.find(Popover).forEach(popover => expect(popover.prop('open')).toBe(false));
});

describe('bound functions to set a & b and passes them to Popover JSX props correctly', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ export default class TraceDiffHeader extends React.PureComponent<Props, State> {
placement="bottomLeft"
title={<TraceIdInput selectTrace={this._diffSetA} />}
content={cohortTableA}
visible={tableVisible === 'a'}
onVisibleChange={this._toggleTableA}
open={tableVisible === 'a'}
onOpenChange={this._toggleTableA}
>
<div className="ub-flex u-flex-1">
<TraceHeader
Expand All @@ -129,8 +129,8 @@ export default class TraceDiffHeader extends React.PureComponent<Props, State> {
placement="bottomLeft"
title={<TraceIdInput selectTrace={this._diffSetB} />}
content={cohortTableB}
visible={tableVisible === 'b'}
onVisibleChange={this._toggleTableB}
open={tableVisible === 'b'}
onOpenChange={this._toggleTableB}
>
<div className="ub-flex u-flex-1">
<TraceHeader
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ exports[`TraceDiffHeader handles absent a & b 1`] = `
selection={Object {}}
/>
}
onVisibleChange={[Function]}
onOpenChange={[Function]}
open={false}
overlayClassName="TraceDiffHeader--popover"
placement="bottomLeft"
title={
Expand All @@ -97,7 +98,6 @@ exports[`TraceDiffHeader handles absent a & b 1`] = `
/>
}
trigger="click"
visible={false}
>
<div
className="ub-flex u-flex-1"
Expand Down Expand Up @@ -198,7 +198,8 @@ exports[`TraceDiffHeader handles absent a & b 1`] = `
selection={Object {}}
/>
}
onVisibleChange={[Function]}
onOpenChange={[Function]}
open={false}
overlayClassName="TraceDiffHeader--popover"
placement="bottomLeft"
title={
Expand All @@ -207,7 +208,6 @@ exports[`TraceDiffHeader handles absent a & b 1`] = `
/>
}
trigger="click"
visible={false}
>
<div
className="ub-flex u-flex-1"
Expand Down Expand Up @@ -312,7 +312,8 @@ exports[`TraceDiffHeader handles absent a 1`] = `
}
/>
}
onVisibleChange={[Function]}
onOpenChange={[Function]}
open={false}
overlayClassName="TraceDiffHeader--popover"
placement="bottomLeft"
title={
Expand All @@ -321,7 +322,6 @@ exports[`TraceDiffHeader handles absent a 1`] = `
/>
}
trigger="click"
visible={false}
>
<div
className="ub-flex u-flex-1"
Expand Down Expand Up @@ -429,7 +429,8 @@ exports[`TraceDiffHeader handles absent a 1`] = `
}
/>
}
onVisibleChange={[Function]}
onOpenChange={[Function]}
open={false}
overlayClassName="TraceDiffHeader--popover"
placement="bottomLeft"
title={
Expand All @@ -438,7 +439,6 @@ exports[`TraceDiffHeader handles absent a 1`] = `
/>
}
trigger="click"
visible={false}
>
<div
className="ub-flex u-flex-1"
Expand Down Expand Up @@ -552,7 +552,8 @@ exports[`TraceDiffHeader handles absent b 1`] = `
}
/>
}
onVisibleChange={[Function]}
onOpenChange={[Function]}
open={false}
overlayClassName="TraceDiffHeader--popover"
placement="bottomLeft"
title={
Expand All @@ -561,7 +562,6 @@ exports[`TraceDiffHeader handles absent b 1`] = `
/>
}
trigger="click"
visible={false}
>
<div
className="ub-flex u-flex-1"
Expand Down Expand Up @@ -676,7 +676,8 @@ exports[`TraceDiffHeader handles absent b 1`] = `
}
/>
}
onVisibleChange={[Function]}
onOpenChange={[Function]}
open={false}
overlayClassName="TraceDiffHeader--popover"
placement="bottomLeft"
title={
Expand All @@ -685,7 +686,6 @@ exports[`TraceDiffHeader handles absent b 1`] = `
/>
}
trigger="click"
visible={false}
>
<div
className="ub-flex u-flex-1"
Expand Down Expand Up @@ -794,7 +794,8 @@ exports[`TraceDiffHeader renders as expected 1`] = `
}
/>
}
onVisibleChange={[Function]}
onOpenChange={[Function]}
open={false}
overlayClassName="TraceDiffHeader--popover"
placement="bottomLeft"
title={
Expand All @@ -803,7 +804,6 @@ exports[`TraceDiffHeader renders as expected 1`] = `
/>
}
trigger="click"
visible={false}
>
<div
className="ub-flex u-flex-1"
Expand Down Expand Up @@ -922,7 +922,8 @@ exports[`TraceDiffHeader renders as expected 1`] = `
}
/>
}
onVisibleChange={[Function]}
onOpenChange={[Function]}
open={false}
overlayClassName="TraceDiffHeader--popover"
placement="bottomLeft"
title={
Expand All @@ -931,7 +932,6 @@ exports[`TraceDiffHeader renders as expected 1`] = `
/>
}
trigger="click"
visible={false}
>
<div
className="ub-flex u-flex-1"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export default class KeyboardShortcutsHelp extends React.PureComponent<Props, St
</Button>
<Modal
title="Keyboard Shortcuts"
visible={this.state.visible}
open={this.state.visible}
onOk={this.onCloserClicked}
onCancel={this.onCloserClicked}
cancelButtonProps={{ style: { display: 'none' } }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ exports[`KeyboardShortcutsHelp renders as expected 1`] = `
}
onCancel={[Function]}
onOk={[Function]}
open={false}
title="Keyboard Shortcuts"
visible={false}
>
<ForwardRef(InternalTable)
className="KeyboardShortcutsHelp--table u-simple-scrollbars"
Expand Down
6 changes: 3 additions & 3 deletions packages/jaeger-ui/src/components/common/CopyIcon.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,17 @@ describe('<CopyIcon />', () => {

it('updates state when tooltip hides and state.hasCopied is true', () => {
wrapper.setState({ hasCopied: true });
wrapper.find(Tooltip).prop('onVisibleChange')(false);
wrapper.find(Tooltip).prop('onOpenChange')(false);
expect(wrapper.state().hasCopied).toBe(false);

const state = wrapper.state();
wrapper.find(Tooltip).prop('onVisibleChange')(false);
wrapper.find(Tooltip).prop('onOpenChange')(false);
expect(wrapper.state()).toBe(state);
});

it('persists state when tooltip opens', () => {
wrapper.setState({ hasCopied: true });
wrapper.find(Tooltip).prop('onVisibleChange')(true);
wrapper.find(Tooltip).prop('onOpenChange')(true);
expect(wrapper.state().hasCopied).toBe(true);
});
});
2 changes: 1 addition & 1 deletion packages/jaeger-ui/src/components/common/CopyIcon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export default class CopyIcon extends React.PureComponent<PropsType, StateType>
<Tooltip
arrowPointAtCenter
mouseLeaveDelay={0}
onVisibleChange={this.handleTooltipVisibilityChange}
onOpenChange={this.handleTooltipVisibilityChange}
placement={this.props.placement}
title={this.state.hasCopied ? 'Copied' : this.props.tooltipTitle}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ exports[`<CopyIcon /> renders as expected 1`] = `
<Tooltip
arrowPointAtCenter={true}
mouseLeaveDelay={0}
onVisibleChange={[Function]}
onOpenChange={[Function]}
placement="top"
title="tooltipTitleValue"
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ exports[`reduxFormFieldAdapter not validated input should render as expected 1`]
exports[`reduxFormFieldAdapter validate input should render Popover as invisible if there is an error but the field is active 1`] = `
<Popover
content="error content"
open={false}
placement="bottomLeft"
title="error title"
visible={false}
>
<ForwardRef
className="AdaptedReduxFormField--isValidatedInput"
Expand All @@ -45,9 +45,9 @@ exports[`reduxFormFieldAdapter validate input should render Popover as invisible
exports[`reduxFormFieldAdapter validate input should render Popover as visible if there is an error and the field is not active 1`] = `
<Popover
content="error content"
open={true}
placement="bottomLeft"
title="error title"
visible={true}
>
<ForwardRef
className="is-invalid AdaptedReduxFormField--isValidatedInput"
Expand All @@ -70,8 +70,8 @@ exports[`reduxFormFieldAdapter validate input should render Popover as visible i

exports[`reduxFormFieldAdapter validate input should render as expected when there is not an error 1`] = `
<Popover
open={false}
placement="bottomLeft"
visible={false}
>
<ForwardRef
className="AdaptedReduxFormField--isValidatedInput"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ describe('reduxFormFieldAdapter', () => {
it('should render Popover as visible if there is an error and the field is not active', () => {
meta.error = error;
const wrapper = shallow(<AdaptedInput input={input} meta={meta} />);
expect(wrapper.find(Popover).prop('visible')).toBeTruthy();
expect(wrapper.find(Popover).prop('open')).toBeTruthy();
expect(wrapper).toMatchSnapshot();
});
});
Expand Down
2 changes: 1 addition & 1 deletion packages/jaeger-ui/src/utils/redux-form-field-adapter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export default function reduxFormFieldAdapter({
</AntInputComponent>
);
return isValidatedInput ? (
<Popover placement="bottomLeft" visible={isInvalid} {...rest.meta.error}>
<Popover placement="bottomLeft" open={isInvalid} {...rest.meta.error}>
{content}
</Popover>
) : (
Expand Down

0 comments on commit 797e0d8

Please sign in to comment.