1418274 - Responsive Design Mode to use ES6 Classes, prop-types and react-dom-factories

Status: RESOLVED FIXED

(DevTools :: Responsive Design Mode, enhancement, P2)

Description

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

There are a number of reasons for this:

1. We need to have the modules available for code from devtools-core, which uses them.
2. It begins the process of getting rid of React deprecation warnings.
3. It prepares us to upgrade to React 16.

Versioning info from devtools-core github repo:

"prop-types": "^15.6.0"
"react-dom-factories": "^1.0.2"

Comment 1

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Bug 1418274 - Responsive Design Mode to ES6 Classes, prop-types and react-dom-factories

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

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8929411 [details]
Bug 1418274 - Responsive Design Mode to ES6 Classes, prop-types and react-dom-factories

https://reviewboard.mozilla.org/r/200736/#review206036

Thanks for working on this!

Overall, the code transformations looks good.  However, bug 1333254 and bug 1319597 just landed which modify several of the components here, so we'll need to rebase this work.  In particular, some components are renamed, so that's something to watch out for.

I'd like to double check it once more when that's done.  I am not aware of other refactoring in progress, so there shouldn't be any more conflicts coming while you revise this work.

::: devtools/client/responsive.html/components/Browser.js:23
(Diff revision 1)
>  
>  const FRAME_SCRIPT = "resource://devtools/client/responsive.html/browser/content.js";
>  
> -module.exports = createClass({
> -
> -  /**
> +class Browser extends PureComponent {
> +  static get propTypes() {
> +    return {

This is an important comment about how this component uses data.  Please restore it somewhere, perhaps as a comment on `propTypes`, since they describe the incoming data.

::: devtools/client/responsive.html/components/DprSelector.js:33
(Diff revision 1)
>      title: value,
>      hidden: true,
>      disabled: true,
>    }, value);
>  
> -module.exports = createClass({
> +class DPRSelector extends PureComponent {

This file was renamed to `DevicePixelRatioSelector.js` by bug 1333254, so we'll need to rebase on top of that.  (The component is also renamed similarly.)

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8929411 [details]
Bug 1418274 - Responsive Design Mode to ES6 Classes, prop-types and react-dom-factories

https://reviewboard.mozilla.org/r/200736/#review206040

(Oops, mis-clicked in MozReview...  Anyway, I'd like see the rebase version, so setting r- for now.)

Comment 4

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 8929411 [details]
Bug 1418274 - Responsive Design Mode to ES6 Classes, prop-types and react-dom-factories

https://reviewboard.mozilla.org/r/200736/#review206822

::: devtools/client/responsive.html/components/Browser.js:23
(Diff revision 1)
>  
>  const FRAME_SCRIPT = "resource://devtools/client/responsive.html/browser/content.js";
>  
> -module.exports = createClass({
> -
> -  /**
> +class Browser extends PureComponent {
> +  static get propTypes() {
> +    return {

Yup, a side affect of dropping displayName... fixed.

Comment 5

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 8929411 [details]
Bug 1418274 - Responsive Design Mode to ES6 Classes, prop-types and react-dom-factories

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

Comment 6

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 8929411 [details]
Bug 1418274 - Responsive Design Mode to ES6 Classes, prop-types and react-dom-factories

https://reviewboard.mozilla.org/r/200736/#review206828

The interdiff is wrong... DprSelector.js no longer exists so you can ignore that section.

Comment 7

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Could you review this before your PTO?

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8929411 [details]
Bug 1418274 - Responsive Design Mode to ES6 Classes, prop-types and react-dom-factories

https://reviewboard.mozilla.org/r/200736/#review206942

Thanks for working on this! :) It looks good to go.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5351572bb1e7
Responsive Design Mode to ES6 Classes, prop-types and react-dom-factories r=jryans

Comment 10

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/5351572bb1e7