Skip to content

Show the vertical ruler in the timeline when hovering the network chart#5548

Merged
fqueze merged 2 commits intofirefox-devtools:mainfrom
fqueze:vertical-ruler-network-panel
Aug 12, 2025
Merged

Show the vertical ruler in the timeline when hovering the network chart#5548
fqueze merged 2 commits intofirefox-devtools:mainfrom
fqueze:vertical-ruler-network-panel

Conversation

@fqueze
Copy link
Contributor

@fqueze fqueze commented Aug 7, 2025

When hovering over the network chart, display a vertical ruler in the timeline to match the behavior of marker chart and stack chart.

This makes it easier to correlate slow network request handling with high CPU use for other reasons in the timeline.

Example profile with multiple network requests and significant CPU use between them: https://share.firefox.dev/3HeQY0D

@fqueze fqueze requested a review from canova August 7, 2025 21:59
@codecov
Copy link

codecov bot commented Aug 7, 2025

Codecov Report

❌ Patch coverage is 88.88889% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.13%. Comparing base (a148dac) to head (a904e4d).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/components/network-chart/index.js 88.88% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5548   +/-   ##
=======================================
  Coverage   86.13%   86.13%           
=======================================
  Files         309      309           
  Lines       29879    29897   +18     
  Branches     8037     8040    +3     
=======================================
+ Hits        25736    25752   +16     
- Misses       3552     3554    +2     
  Partials      591      591           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

When hovering over the network chart, display a vertical ruler in the
timeline to match the behavior of marker chart and stack chart.

This makes it easier to correlate slow network request handling with
high CPU use for other reasons in the timeline.
@fqueze fqueze force-pushed the vertical-ruler-network-panel branch from f23d084 to a904e4d Compare August 8, 2025 14:27
Copy link
Member

@canova canova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'm surprised that we don't use withChartViewport in this component but it looks like the way we handle viewport is different here, and didn't spend a lot of time to understand why. But considering all this, the code looks good to me.

@fqueze fqueze enabled auto-merge (squash) August 12, 2025 13:16
@fqueze fqueze merged commit 927870f into firefox-devtools:main Aug 12, 2025
12 of 13 checks passed
@mstange
Copy link
Contributor

mstange commented Aug 12, 2025

Yeah we really should start using it there. The network chart currently doesn't support pinch-zoom and panning, and it would be nice if it did.

@canova
Copy link
Member

canova commented Aug 12, 2025

Ah that's a good point. I filed #5552 for that.

@canova canova mentioned this pull request Sep 2, 2025
canova added a commit that referenced this pull request Sep 2, 2025
Changes:

[Nazım Can Altınova] Display the marker description at the top inside
the marker tooltips (#5534)
[Florian Quèze] Change the 'JavaScript' radio button label to 'Script'
(#5530)
[Markus Stange] Implement profile logic and some selectors for the
function list (#5525)
[Markus Stange] Some small type fixes (#5538)
[Markus Stange] Simplify return type of the callback we pass to
setState. (#5540)
[Markus Stange] Pass the correct value to the reducer's action argument
(#5543)
[Markus Stange] Change withSize to accept PropsWithoutSize as its type
parameter (#5541)
[Nazım Can Altınova] Make sure that the test-debug command runs the
tests properly (#5545)
[Markus Stange] Improve type coverage involving network phases (#5539)
[Markus Stange] Change implementation of withChartViewport (#5542)
[Florian Quèze] A new permalink should be generated and shown after
using the re-upload feature. (#5547)
[Florian Quèze] Show the vertical ruler in the timeline when hovering
the network chart (#5548)
[Markus Stange] Convert the entire codebase to TypeScript (#5549)
[Nazım Can Altınova] Update the yarn.lock file after recent changes
(#5557)
[Markus Stange] Add proper TypeScript coverage for window-navigation.ts
(#5559)
[Markus Stange] Remove leftover $FlowExpectError comments (#5560)
[Markus Stange] Fix Iterator / Iterable confusion (#5561)
[Nazım Can Altınova] Remove the unneeded test-all:ci script (#5566)
[Nazım Can Altınova] Fix a type case inconsistency (#5569)
[Florian Quèze] Make 'yarn lint --fix' work as an alias to 'yarn
lint-fix'. (#5563)
[Ryan Hunt] Don't stringify JSON again in fetchUrlResponse (#5570)
[Nazım Can Altınova] Upgrade ESLint to version 9 (#5567)
[Markus Stange] Simplify Worker setup, and support .json.gz inputs in
symbolicator-cli (#5556)
[Nazım Can Altınova] Add TypeScript coverage to the intersection
observer mock (#5574)
[Markus Stange] Set the preview selection to null when there is no
selection (#5568)
[Markus Stange] Add tests for query-api.ts (#5571)
[Markus Stange] Enable noUnusedParameters and
@typescript-eslint/no-unused-vars and clean up a few more things (#5576)
[Ryan Hunt] Embed iongraph-web and use for iongraph.json source files
(#5577)
[Markus Stange] Remove recursion in filterThreadToSearchString (#5572)


And thanks to our localizers:

be: Mikalai Udodau
de: Michael Köhler
el: Jim Spentzos
en-CA: chutten
en-GB: Paul
es-CL: ravmn
fr: Théo Chevalier
fur: Fabio Tomat
fy-NL: Fjoerfoks
ia: Melo46
it: Francesco Lodolo [:flod]
nl: Mark Heijl
pt-BR: Marcelo Ghelman
ru: Valery Ledovskoy
sv-SE: Luna Jernberg
tr: Rua
tr: Selim Şumlu
uk: Іhor Hordiichuk
zh-CN: Olvcpr423
zh-TW: Pin-guang Chen
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants