Closed Bug 1399909 Opened 6 years ago Closed 6 years ago

Prevent markupview resize during inspector startup

Categories

(DevTools :: Inspector, enhancement, P3)

Product:
DevTools
Component:
Inspector
Type:
enhancement

Tracking

(firefox57 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: ochameau, Assigned: ochameau)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Bug 1399909 - Prevent the inspector resizing its left pane during load.
59 bytes, text/x-review-board-request
gl
: review+
Details 
Today, the inspector load is still a bit clunky.
Things blink, resizes, ...

The left panel of the inspector, containing its tabbar, the markup view and the breadcrumb starts loading with full panel width. Then, once the rule view (or any other sidebar tool) is ready, it is resized.
We should prevent that to make the inspector load more natural.

After this patch:
* various parts of the inspector still load in parallel.
  We may try to display more things at once.
  Like waiting for markup view and rule view to be ready before displaying things?
* the sidebar toggle button is still loaded very late and blinks
* we briefly see toolbox background sometimes, that introduces a blink effect.
Just a note the sidebar toggle button is suppose to go away with the photon redesign in Bug 1396349. It was deemed too big of a change for 57 and will probably land afterwards.
Oh cool! Can't wait to be on 58 then!
Comment on attachment 8908204 [details]
Bug 1399909 - Prevent the inspector resizing its left pane during load.

https://reviewboard.mozilla.org/r/179868/#review185096

::: devtools/client/shared/components/splitter/split-box.js:66
(Diff revision 1)
>        vert: this.props.vert,
>        width: this.props.initialWidth || this.props.initialSize,
>        height: this.props.initialHeight || this.props.initialSize
>      };
>    },
>  

This should be moved after componentWillReceiveProps to make it flow with the natural React component lifecycle.
Attachment #8908204 - Flags: review+
Attachment #8908204 - Flags: review?(pbrosset)
Can't land because of this test failure:
[task 2017-09-15T07:35:31.773783Z] 07:35:31    ERROR -  60 INFO TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_accessibility-01.js | The selected item in the requests menu was incorrect. - Got -1, expected 1

Would it be because of split-box change??
Assignee: nobody → poirot.alex
status-firefox57: --- → fix-optional
Priority: -- → P3
It was the modification made to split-box.js
I thought props would not change, but it does for this netmonitor test...

Does shouldComponentUpdate still look good?
Flags: needinfo?(gl)
(In reply to Alexandre Poirot [:ochameau] from comment #8)
> It was the modification made to split-box.js
> I thought props would not change, but it does for this netmonitor test...
> 
> Does shouldComponentUpdate still look good?

Looks good.
Flags: needinfo?(gl)
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/930533fdd67d
Prevent the inspector resizing its left pane during load. r=gl
https://hg.mozilla.org/mozilla-central/rev/930533fdd67d
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox57: fix-optional → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.