Skip to content

Commit

Permalink
Rerender Maybe components when they're first disabled (#2617)
Browse files Browse the repository at this point in the history
\#2395 ended up being caused by this since we don't rerender placeholder
components created by `st.empty()` when react props/state change as a
performance optimization, but we need to do so when replacing an
existing component with an empty one to get rid of what's currently
drawn on screen.

Closes #2395
  • Loading branch information
vdonato committed Jan 21, 2021
1 parent 419c858 commit bd41a46
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
11 changes: 11 additions & 0 deletions frontend/src/components/core/Maybe/Maybe.test.tsx
Expand Up @@ -56,6 +56,17 @@ describe("The Maybe component", () => {
expect(spyShouldComponentUpdate).toHaveBeenCalled()
expect(spyRender).toHaveBeenCalled()
})

it("should call render() when a Maybe is first disabled", () => {
const spyShouldComponentUpdate = jest.spyOn(
Maybe.prototype,
"shouldComponentUpdate"
)
const spyRender = jest.spyOn(Maybe.prototype, "render")
component.setProps({ name: "new name", enable: false })
expect(spyShouldComponentUpdate).toHaveBeenCalled()
expect(spyRender).toHaveBeenCalled()
})
})

describe("when enable is false", () => {
Expand Down
5 changes: 4 additions & 1 deletion frontend/src/components/core/Maybe/Maybe.tsx
Expand Up @@ -30,7 +30,10 @@ class Maybe extends React.Component<Props, State> {
nextState: Readonly<State>,
nextContext: any
): boolean {
return nextProps.enable
// We have our component update if either props.enable or nextProps.enable
// is true to ensure that we rerender in the case that an Element is
// removed by replacing it with an empty one (so goes from enabled->disabled).
return this.props.enable || nextProps.enable
}

public render(): React.ReactNode {
Expand Down

0 comments on commit bd41a46

Please sign in to comment.