Skip to content

ui: Refactor Flame Graph color mapping logic#5954

Merged
yomete merged 1 commit into
mainfrom
mappings-flamegraph
Sep 15, 2025
Merged

ui: Refactor Flame Graph color mapping logic#5954
yomete merged 1 commit into
mainfrom
mappings-flamegraph

Conversation

@yomete

@yomete yomete commented Sep 15, 2025

Copy link
Copy Markdown
Contributor

There was an opportunity to consolidate how the mapping files and filenames are used to derive the colors to be used for the Flame Graph.

This PR helps to ensure we're not repeating logic in different components by using the useProfileMetadata and useVisualizationState hooks to get all the coloring data needed and passing it down as props to the components that needs them.

It also fixes a bug where if you switch the coloring by from filename to binary and vice versa, the Flame Graph is not immediately re-rendered with the new colors.

@yomete
yomete requested a review from a team as a code owner September 15, 2025 14:03
@alwaysmeticulous

alwaysmeticulous Bot commented Sep 15, 2025

Copy link
Copy Markdown

✅ Meticulous spotted visual differences in 17 of 590 screens tested, but all differences have already been approved: view differences detected.

Meticulous evaluated ~5 hours of user flows against your PR.

Last updated for commit 225b3a0. This comment will update as new commits are pushed.

prevProps.totalWidth === nextProps.totalWidth &&
prevProps.height === nextProps.height &&
prevProps.effectiveDepth === nextProps.effectiveDepth
prevProps.effectiveDepth === nextProps.effectiveDepth &&

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This part fixes the bug where the Flame Graph doesn't get redrawn/recolored when changing the color by value.

@manojVivek manojVivek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lgtm!

@yomete
yomete merged commit e22ffd3 into main Sep 15, 2025
37 of 38 checks passed
@yomete
yomete deleted the mappings-flamegraph branch September 15, 2025 15:09
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.

2 participants