Closed Bug 1328567 Opened 6 years ago Closed 6 years ago

TreeView expandedNodes should update when props changed

Categories

(DevTools :: Shared Components, defect, P1)

defect

Tracking

(firefox53 fixed)

RESOLVED FIXED
Firefox 53
Iteration:
53.4 - Jan 9
Tracking Status
firefox53 --- fixed

People

(Reporter: rickychien, Assigned: rickychien)

References

(Blocks 1 open bug)

Details

(Whiteboard: [netmonitor])

Attachments

(1 file)

I ran into this issue when using TreeView in ResponsePanel (bug 1317651).

Current TreeView will accept an expandedNodes prop in getInitialState:

expandedNodes: this.props.expandedNodes,

Expanded nodes will not be updated after component mount if we pass new expandedNodes prop into TreeView because there is no way to update this.state.expandedNodes state.

It's a necessary since ResponsePanel will update TreeView object prop along with new expandedNodes prop depends on selected request. So we should fix this by triggering a setState in componentWillReceiveProps.
Comment on attachment 8823576 [details]
Bug 1328567 - TreeView expandedNodes should update when props changed

https://reviewboard.mozilla.org/r/102114/#review102462

::: devtools/client/shared/components/tree/tree-view.js:132
(Diff revision 1)
> +        this.setState(Object.assign({}, this.state, {
> +          expandedNodes: new Set([...nodes, ...expandedNodes]),
> +        }));

I think it should be rather:

this.setState(Object.assign({}, this.state, {
  expandedNodes,
}));

The consumer should be responsible for joining previously expanded nodes with new set of nodes if needed.

Does that work for your case?

Honza
Iteration: --- → 53.4 - Jan 9
Flags: qe-verify?
Priority: -- → P1
Whiteboard: [netmonitor][triage] → [netmonitor]
Yes, it works for my case! I updated my patch, please take a look again.
Comment on attachment 8823576 [details]
Bug 1328567 - TreeView expandedNodes should update when props changed

https://reviewboard.mozilla.org/r/102114/#review102558

R+, thanks!

Honza
Attachment #8823576 - Flags: review?(odvarko) → review+
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4903a05dfaec
TreeView expandedNodes should update when props changed r=Honza
https://hg.mozilla.org/mozilla-central/rev/4903a05dfaec
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Flags: qe-verify? → qe-verify-
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.