1443121 - Use new redux-connect wrapper for the Console

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P2)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Bug 1419350 introduced a helper function that wraps Redux's `connect()` method and uses `VisibilityHandler` component as HOC. This construct can be consequently used to ignore Redux updates to avoid rendering when a panel is running in background.

This helper should be also used in the Console panel (ConsoleOutput component)

The helper should go into to be easily shared:

devtools/client/shared/redux/redux-connect.js file

Honza

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Bug 1443121 - Use new redux-connect wrapper for the Console;

Review commit: https://reviewboard.mozilla.org/r/224990/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/224990/

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8956046 [details]
Bug 1443121 - Use new redux-connect wrapper for the Console;

https://reviewboard.mozilla.org/r/224990/#review230934

A couple of minor comments, but this is good to go :)

::: commit-message-69f9a:1
(Diff revision 1)
> +Bug 1443121 - Use new redux-connect wrapper for the Console; r=nchevobbe
> +

maybe add that the the file was renamed and that netmonitor now requires it with an absolute path

::: devtools/client/shared/redux/moz.build:12
(Diff revision 1)
>  DIRS += [
>      'middleware',
>  ]
>  
>  DevToolsModules(
> +    'connect.js',

nit: Perhaps we could name it visibility-aware-connect.js so it's more obvious  how it is different from the regular react-redux's connect ?

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8956046 [details]
Bug 1443121 - Use new redux-connect wrapper for the Console;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/224990/diff/1-2/

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #2)
> maybe add that the the file was renamed and that netmonitor now requires it
> with an absolute path
Done

> nit: Perhaps we could name it visibility-aware-connect.js so it's more
> obvious  how it is different from the regular react-redux's connect ?
Done

I used `visibility-handler-connect.js` since the implementation
is based on `VisibilityHandler` component.

Thanks Nicolas!
Honza

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/55b683e679b0
Use new redux-connect wrapper for the Console; r=nchevobbe

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/55b683e679b0