Closed Bug 1836439 Opened 1 year ago Closed 1 year ago

Tab react components render at almost any state change

Categories

(DevTools :: Debugger, enhancement)

enhancement

Tracking

(firefox116 fixed)

RESOLVED FIXED
116 Branch
Tracking Status
firefox116 --- fixed

People

(Reporter: ochameau, Assigned: ochameau)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

The Tabs and Tab components currently re-render for almost all state changes because of lack of correct memoization of selectors, but also because <Tab> is being given props which always change (onDrag* event handler). Then they are all updated unecessarily because they have selectedLocation as prop, whereas it only impact deselected and newly selected tabs only.

This was calling outdated selector name and wrong argument were passed to the action.

While I'm reading this code, also fix disabled state of two other context menus.
And also disable them based on Tab source rather than the selected source!

Assignee: nobody → poirot.alex
Status: NEW → ASSIGNED

This isn't enough to stop all unecessary Tab updates, but may help cut a few.
Each time Tabs re-rendered, all Tab children component were forced to re-render
because all these onDrag* props were new objects.

Computing the "isActive" field from the selector allows to update the component
only when the related source becomes selected or stops being selected.

Also, activeSearch being set to "source" no longer exists.

Last but not least, tweak Tab's key property to be unique per tab.
Using 'index' is wrong, when moving or adding tab first, it will change all Tab's state!

With all these patches, now, when you have more than two tabs,
when selecting another tab, only two Tab components gets re-rendered:
the old and the new being selected.

Sometimes the UI code emit an undefined new index.

Blocks: 1836929
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/df036e39dd8c
[devtools] Fix copy to clipboard tab context menu. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/d36f1aac82d0
[devtools] Stop forcing Tab components to always rerender because of arrow function passed in props. r=devtools-reviewers,bomsy
https://hg.mozilla.org/integration/autoland/rev/514dc1087d5b
[devtools] Move Tab context menu from React component to an action. r=devtools-reviewers,bomsy
https://hg.mozilla.org/integration/autoland/rev/6fb412d1fc55
[devtools] Stop updating all Tab components on selected location change. r=devtools-reviewers,bomsy
https://hg.mozilla.org/integration/autoland/rev/bd2bde7ae3a9
[devtools] Avoid unecessary state change when dragging tabs. r=devtools-reviewers,bomsy
Blocks: 1839347
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: