Move window topbar content to menu on small screens #3872
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3872 +/- ##
==========================================
+ Coverage 94.91% 94.94% +0.02%
==========================================
Files 323 325 +2
Lines 16136 16237 +101
Branches 2531 2551 +20
==========================================
+ Hits 15316 15416 +100
- Misses 815 816 +1
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
- Keyboard navigation does not work in the small variant yet. I don't know how relevant keyboard navigation is for very small screens (i.e. probably phones), but I'd assume it's still an accessibility requirement.
- The way this is implemented results in a change of order of the buttons.
- up until now, the order was
- Window view / thumbnail display
- Plugin area
- Maximize / restore window
- Full screen
- Close Window
- The plugins / more button has now moved behind Maximize and Fullscreen. The question is if the latter two should better be kept together with the Close button, but this would make the change a little more complicated. I'd really like a bit more feedback on that.
- up until now, the order was
Edit, for illustration...
Before:
After:
| const config = getWindowConfig(state, { windowId }); | ||
|
|
||
| return {}; | ||
| }; |
There was a problem hiding this comment.
mapStateToProps looks superfluous and like an accidental copy&paste. Calling connect(null, mapDispatchToProps) should be fine in enhance.
| /** | ||
| * WindowTopBarMenu | ||
| */ | ||
| export function WindowTopBarMenu({ |
There was a problem hiding this comment.
This is a minor thing, but I'm not sure if WindowTopBarMenu is the best name for this component, as it contains the entire content of the WindowTopBar aside from the sidebar button. Not sure what a better name would be. WindowTopBarContent? WindowTopBarControls?
This is an attempt to address #2891