1336382 - Implement RequestListContextMenu React component

Status: RESOLVED WONTFIX

(DevTools :: Netmonitor, defect, P3)

Description

[Ricky Chien [:rickychien] (inactive)]

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

Comment 2

[Ricky Chien [:rickychien] (inactive)]

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

Comment 3

[Ricky Chien [:rickychien] (inactive)]

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.