Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Devtools React Profiler: Shrink flamegraph window's width to make room for scrollbar #17339

Closed
wants to merge 1 commit into from

Conversation

gejimayu
Copy link

@gejimayu gejimayu commented Nov 11, 2019

Hide scrollbar to appear on flame graph React Profiler.

The reason is that the scrollbar seems to block some of the flame graph. In addition to that, the scrollbar does not serve any purpose.

This is the issue #16550

Before

before

After / Results

Chrome
chrome

Firefox
firefox

@sizebot
Copy link

sizebot commented Nov 11, 2019

No significant bundle size changes to report.

Generated by 🚫 dangerJS against 4a26fe2

@gejimayu gejimayu changed the title Devtools: Hide scrollbar on flame graph React Profiler DevTools: Hide scrollbar on flame graph React Profiler Nov 11, 2019
@gejimayu
Copy link
Author

@bvaughn may i ask for review? thanks in advance

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

I'm sorry, but we can't make this change. The scrollbar does serve a purpose, although maybe less of one for track pad users.

@gejimayu
Copy link
Author

gejimayu commented Nov 26, 2019

Hi thanks for the response.
I see, understood. What if i shrink the window width to make room for scrollbar?

@gejimayu
Copy link
Author

Done.
Chrome
chrome
Firefox
firefox

@gejimayu gejimayu changed the title DevTools: Hide scrollbar on flame graph React Profiler Devtools React Profiler: Shrink flamegraph window's width to make room for scrollbar Nov 26, 2019
@bvaughn
Copy link
Contributor

bvaughn commented Nov 26, 2019

I think the current scrollbar UX is fine. It only shows when scrolling is active, then it hides itself again. Permanently shrinking the width would also negatively impact non-Mac/trackpad users since the scrollbar on their systems takes up fixed space already.

I appreciate the proposal and sharing the idea 😄 but I don't think we should change anything in this panel. Sorry!

@bvaughn bvaughn closed this Nov 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants