Closed
Bug 1336382
Opened 8 years ago
Closed 7 years ago
Implement RequestListContextMenu React component
Categories
(DevTools :: Netmonitor, defect, P3)
DevTools
Netmonitor
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: rickychien, Assigned: rickychien)
References
(Blocks 1 open bug)
Details
Architecture reactify step for creating top level NetworkMonitor react component.
* Implement RequestsListContextMenu component instead of requests-list-context-menu.js
* Remove requests-list-context-menu.js
Updated•8 years ago
|
Flags: qe-verify?
Updated•8 years ago
|
Flags: qe-verify? → qe-verify+
Priority: -- → P3
QA Contact: ciprian.georgiu
Whiteboard: [netmonitor][triage] → [netmonitor-reserve]
Updated•8 years ago
|
Summary: Implement RequestListContextMenu component → Implement RequestListContextMenu React component
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Comment 2•7 years ago
|
||
After experiment, I've some different opinions of converting contextmenu into React component.
ContextMenu is built on menu.js
* launchpad https://github.com/devtools-html/devtools-core/blob/master/packages/devtools-contextmenu/menu.js
* toolbox https://searchfox.org/mozilla-central/source/devtools/client/framework/menu.js (using XUL native menupopup)
So that we can handle popup show & popup hide more precisely like nature contextmenu.
For example, when contextmenu is showing on screen, mouse click on an area where out of contextmenu itself can hide contextmenu, which behavior is able to be handled by menu.js properly. But if we want to convert exist requests-list-context-menu into react, it forces us to re-implement those native supported behaviors on our own. And also introduce extra state or Redux props for contextmenu open.
React component ends up expecting an element return in render() function, and hookup the component in somewhere (in our case, the contextmenu component would be appended in RequestListContent.js). It's contrary to our case because in reality our popup element has already appended on document [1][2]. Therefore, it would cause an unnecessary setup such as rendering an extra empty div in RequestsListContextMenu's render function.
[1] https://searchfox.org/mozilla-central/source/devtools/client/framework/menu.js#67
[2] https://github.com/devtools-html/devtools-core/blob/master/packages/devtools-contextmenu/menu.js#L39
Assignee | ||
Comment 3•7 years ago
|
||
Close this issue due to agreement in today's netmonitor meeting due to comment 2.
I'll file a new bug for arranging left over files (xxx-context-menu.js...etc) under src folder.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: qe-verify+
Resolution: --- → WONTFIX
Whiteboard: [netmonitor-reserve]
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•