Closed
Bug 1328567
Opened 8 years ago
Closed 8 years ago
TreeView expandedNodes should update when props changed
Categories
(DevTools :: Shared Components, defect, P1)
DevTools
Shared Components
Tracking
(firefox53 fixed)
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 hidden (mozreview-request) |
Comment 2•8 years ago
|
||
mozreview-review |
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
Updated•8 years ago
|
Blocks: netmonitor-html
Iteration: --- → 53.4 - Jan 9
Flags: qe-verify?
Priority: -- → P1
Whiteboard: [netmonitor][triage] → [netmonitor]
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•8 years ago
|
||
Yes, it works for my case! I updated my patch, please take a look again.
Comment 5•8 years ago
|
||
mozreview-review |
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
Comment 7•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Updated•8 years ago
|
Flags: qe-verify? → qe-verify-
Updated•7 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•