Closed Bug 1411160 Opened 7 years ago Closed 7 years ago

Adds an initial react/redux template for loading a Changes View in the inspector sidebar

Categories

(DevTools :: Inspector, enhancement, P3)

enhancement

Tracking

(firefox58 fixed)

RESOLVED FIXED
Firefox 58
Tracking Status
firefox58 --- fixed

People

(Reporter: gl, Assigned: gl)

References

Details

(Whiteboard: [designer-tools][stockwell fixed:product])

Attachments

(2 files)

This adds an initial react/redux template for loading a Changes sidepanel in the inspector sidebar.
Blocks: 1411161
I looked at this very very quickly, it looks good, pretty similar to the events view you're also working on in bug 1411161.

However before going any further, I'd like to discuss about this:
Victoria has worked on a rough wireframe for this and it involves showing a changes counter in the toolbox toolbar instead. So adding a new sidebar tab wouldn't be compatible with this. I'll attach a screenshot later.

What are your thoughts about this?
I think the most complex piece of work for this feature is server-side, and that can be worked on however we decide to show things in the toolbox. So maybe that means we should start with that?
Do we need to think about an export mechanism from the start? Or just a view that shows the changes?
Victoria's wireframe is based on the existence of an export feature. Because you can't see the changes. Your approach is more gradual I think where, we just show the changes in one place.

My suggestion is the following: if adding this piece of UI (the sidebar) is really cheap, then let's do it because it gives us a visual way to test that the server-side implementation works.
If it's going to take quite a lot of work, I'd say let's drop it for now and work on the server-side first.
I think we will need a client side view to test the server-side implementation. I imagine more of a view similar to Edge DevTools seen here https://docs.microsoft.com/en-us/microsoft-edge/f12-devtools-guide/elements/changes. I assume the mockups are still a gradual wip and should not block prototyping this since the client side implementation should be really visualizing the server side implementation. I also assume there is some view based on the mockup that there is a view to display the changes/diff so this shouldn't be totally incompatible.
Sounds good to me. You're totally right, the mockup is a WIP for now, and if that UI is going to make it easy for you to prototype the server-side stuff, I'm happy to R+.
Comment on attachment 8921348 [details]
Bug 1411160 - Adds an initial react/redux template for loading a Changes View in the inspector sidebar.

https://reviewboard.mozilla.org/r/192382/#review198604
Attachment #8921348 - Flags: review?(pbrosset) → review+
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b44af7ae9208
Adds an initial react/redux template for loading a Changes View in the inspector sidebar. r=pbro
Backed out for build bustage (duplicate files) - and also bug 1411161 because it depended on this bug:

https://hg.mozilla.org/integration/mozilla-inbound/rev/3a3f1669867c0a41904ab445d2a585533463b92c

Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=b44af7ae9208399a79d178b6ce392da0282b5d31&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=retry&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception
Build log: https://treeherder.mozilla.org/logviewer.html#?job_id=139919289&repo=mozilla-inbound

[task 2017-10-26T17:02:31.525Z] 17:02:31     INFO -  WARNING: Found 20 duplicated files taking 36667 bytes (uncompressed)
[task 2017-10-26T17:02:31.526Z] 17:02:31     INFO -  ERROR: The following duplicated files are not allowed:
[task 2017-10-26T17:02:31.526Z] 17:02:31     INFO -  browser/chrome/devtools/modules/devtools/client/inspector/changes/actions/index.js
[task 2017-10-26T17:02:31.526Z] 17:02:31     INFO -  browser/chrome/devtools/modules/devtools/client/inspector/flexbox/actions/index.js
[task 2017-10-26T17:02:31.526Z] 17:02:31     INFO -  /builds/worker/workspace/build/src/toolkit/mozapps/installer/packager.mk:36: recipe for target 'stage-package' failed
[task 2017-10-26T17:02:31.526Z] 17:02:31     INFO -  gmake[7]: *** [stage-package] Error 1
Flags: needinfo?(gl)
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e4cadda2d01
Adds an initial react/redux template for loading a Changes View in the inspector sidebar. r=pbro
Backed out bug 1411161 and bug 1411160 for build bustage (duplicate files):

https://hg.mozilla.org/integration/mozilla-inbound/rev/8e4cadda2d0129e0cfff9f4e7da16595408a2bde
https://hg.mozilla.org/integration/mozilla-inbound/rev/2db16a02d6d10ca6c9b3135f8d4f6f0bca0a8fce

Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=2db16a02d6d10ca6c9b3135f8d4f6f0bca0a8fce&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=retry&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception
Build log: https://treeherder.mozilla.org/logviewer.html#?job_id=139985262&repo=mozilla-inbound

[task 2017-10-26T20:31:17.345Z] 20:31:17     INFO -  WARNING: Found 20 duplicated files taking 36593 bytes (uncompressed)
[task 2017-10-26T20:31:17.346Z] 20:31:17     INFO -  ERROR: The following duplicated files are not allowed:
[task 2017-10-26T20:31:17.346Z] 20:31:17     INFO -  browser/chrome/devtools/modules/devtools/client/inspector/changes/types.js
[task 2017-10-26T20:31:17.346Z] 20:31:17     INFO -  browser/chrome/devtools/modules/devtools/client/inspector/events/types.js
[task 2017-10-26T20:31:17.346Z] 20:31:17     INFO -  /builds/worker/workspace/build/src/toolkit/mozapps/installer/packager.mk:36: recipe for target 'stage-package' failed
[task 2017-10-26T20:31:17.346Z] 20:31:17     INFO -  gmake[7]: *** [stage-package] Error 1
Backout by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3705f52f7234
Backed out changeset 8e4cadda2d01 for build bustage due to duplicate files. r=backout
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/020ee4090c95
Adds an initial react/redux template for loading a Changes View in the inspector sidebar. r=pbro
Flags: needinfo?(gl)
https://hg.mozilla.org/mozilla-central/rev/020ee4090c95
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Whiteboard: [designer-tools] → [designer-tools][stockwell fixed:product]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: