Skip to content
This repository was archived by the owner on Jun 26, 2020. It is now read-only.

React Rally profiler feedback #1100

Merged
merged 15 commits into from
Aug 20, 2018
Merged

React Rally profiler feedback #1100

merged 15 commits into from
Aug 20, 2018

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Aug 18, 2018

Issue #1099

Demo build available at react-devtools-profiler-prerelease.now.sh

Flamegraph

  • 49b9e47, 5cf07b2: Flamegraph/ranked charts should enable you to deselect the current fiber so the right hand pane shows commit statistics again. Perhaps this should be a new button in the top of the panel. Clicking in an "empty" space of the flame graph should also deselect.
  • e03c7da: Show un-rounded durations on hover in tooltips for more info

Horizontal snapshot selector

  • d5047c9, ea8e92c: Make the selected commit more prominent in the horizontal selector than just an alpha transform. Too many people didn't notice it. Maybe add an overlay pattern so it stands out more?
  • ad38466: Left/right arrow keys should step through commits/snapshots.
  • a2907c8: Make sure jumping from fiber-durations bar chart view to flame/ranked also horizontally scrolls the snapshot selector.
  • 0c79424: When dragging, don't leave the dragging state until you release the mouse. (Even if you mouse outside of the selector.)

Right hand details pane

  • 5cf07b2: Make the right hand details pane buttons blue so they stand out more as buttons.
  • 6f075a5: Display "committed at" rather than "commit time".

Component durations bar chart

  • 5e7de4e: It's not obvious which commits are visible in the component durations bar chart. Perhaps we could dim the commits thate are not visible in the above selector?

Elements tree view

  • e881623: Single-click on the tree view toggle arrow doesn't expand/collapse the tree anymore. (You have to double-click it.)

General

  • 27e1f29: "Show native nodes" probably isn't that useful– maybe add a settings/gear icon there and have a profiling preferences popup?
  • 802932b, e8c4ecb: Add a preference (in the new profiler preferences popup) to filter commits below a certain duration too (to reduce noise in commit selector).

@bvaughn bvaughn changed the title React Rally profiler feedback [WIP] React Rally profiler feedback Aug 19, 2018
@bvaughn bvaughn requested a review from gaearon August 19, 2018 16:35
@bvaughn
Copy link
Contributor Author

bvaughn commented Aug 19, 2018

This doesn't resolve all of the requests in #1099, but I think it hits most of the low hanging fruit ones. I'll follow up on some of the others later.

If you don't have the time/energy to review this PR, I feel comfortable enough about it to just merge as-is. Just wanted to offer before plowing forward. 😄

@bvaughn bvaughn merged commit 68a272d into facebook:master Aug 20, 2018
@bvaughn bvaughn deleted the issues/1099 branch August 20, 2018 14:31
OrDuan pushed a commit to OrDuan/react-devtools that referenced this pull request Aug 23, 2018
* Click in empty space of flamegraph/ranked chart to deselect an element

* Added close/stop-inspecting button to fiber detail pane

* Show unrounded durations on hover for chart tooltips

* Make selected commit more prominent in snapshot selector

* Left/right arrow keys navigate selected snapshot/commit

* Snapshot selector auto-scrolls to ensure selected commit is visible

* Snapshot selector shows filtered commit list when inspecting fiber durations

* Tweaked selected snapshot style again to stand out more

* Single click on arrow toggles tree open/closed

* Moved 'show native elements?' option into new Profiler settings panel

* "Commit time" -> "Committed at"

* Left pad number to avoid reflow when stepping through snapshots

* When dragging, don't leave the dragging state until you release the mouse. (Even if you mouse outside of the selector.)

* Add control to filter commits below a thresold (ms)

* Commit filtering applies to snapshot durations view as well
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants