Add a fullscreen button to the bottom box
Fixes #5583. I wasn't sure where to get an icon for this, so I just re-used an existing one. I can look for something better if you have a preferred source for them.
Codecov Report
:x: Patch coverage is 64.70588% with 12 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 85.60%. Comparing base (2ff1b40) to head (dd0b8cd).
:warning: Report is 3 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #5605 +/- ##
==========================================
- Coverage 85.63% 85.60% -0.03%
==========================================
Files 312 313 +1
Lines 30892 30927 +35
Branches 8420 8519 +99
==========================================
+ Hits 26453 26475 +22
- Misses 4009 4022 +13
Partials 430 430
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
Looks pretty nice! But I think we should add more ways to exit this view.
I can think of these three ways that users might try to get back out of a view that takes over the entire page:
- Navigate back
- Close the tab
- Press Esc
I actually remember myself closing profiler tabs unintentionally in the old "cleopatra" Firefox profile viewer which actually had this type of fullscreen source view, because the context cues somehow made me think I was in a new tab.
So I think we should at least make the "navigate back" interaction work, maybe also the "Press Esc" interaction. For navigating back to un-maximize the source view, the source view maximize state just needs to be stored in the URL state.
It would be cool to have this button open the source code in a new window, so that you can close the tab and not lose your profile. But that's probably a bit complicated. I wonder if we can open an about:blank window, put some minimal DOM in there with document.open/write/close and render into it with ReactDOM. Here's an article I found which describes doing something similar in Electron: https://pietrasiak.com/creating-multi-window-electron-apps-using-react-portals
Anyway, the only requirement I have for now is to make it so that navigating back un-maximizes.
So I think we should at least make the "navigate back" interaction work, maybe also the "Press Esc" interaction. For navigating back to un-maximize the source view, the source view maximize state just needs to be stored in the URL state
I will go for this. The portal trick and separate window seems neat, but I'm a bit concerned it's above my React skill level.
Not localization, but if the icon has an arrow going out, it should change for the opposite action (restore). With that said, I think that icon is used for "this link opens in a new window" (even with Profiler), so that's confusing
We should probably use a different icon. I just picked an existing one to have something while testing this out. @mstange or @flodolo do you have a recommendation on where to get a new icon for this?
I've updated the patch with new messages, new SVG icons for minimize/maximise (from font awesome, license here I think), escape closes fullscreen, and using the history back button closes fullscreen.
Light review ping @mstange
I tried out both of the icons, I'm not sure which I prefer. I also moved the icon to be next to close button, as I think it logically goes there.
Here's the photon one:
And here's the acorn one:
The PR here has the acorn one now. Let me know which you prefer and I can rollback to the photon one easily.
I also changed it so that closing the bottom box will remove the fullscreen state. I did this by dispatching two actions in this case because I wasn't sure how to have a single reducer update the state for both in the current redux architecture. Let me know if there's a better way to do it.
Friendly review ping @canova
@flodolo I believe the changes you requested have been applied, could you approve?