-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
feat: Hide keyboard shortcuts on mobile screens #9051
Conversation
This fixes a regression I introduced in https://github.com/WordPress/gutenberg/pull/8756/files#diff-dcb8eefee4aaf82c9ddb042494547527R158.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This improves on the menu situation on small viewports, and it also helps (hot)fix a regression I introduced. Sorry about that. Let's get this into 3.6.
// Hide the keyboard shortcuts on mobile, because they aren't super-useful | ||
// for most mobile users and it frees up screen space for any item | ||
// with a long description. | ||
display: none; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably fine for now, but I wonder if this should be hidden entirely in the generic MenuItem component or something specific to the Global menu item.
It can be used in the Block Settings Menu and elsewhere since it's a generic component.
I think hidden globally makes sense. Sorry, that was deliberate, I should’ve mentioned. My patch is global, right? 😅
Most mobile viewports happen on devices with keyboards that don’t support shortcuts, so hiding that information entirely to reduce noise and also give more space to other info on a screen with reduced real estate seems a good rule globally to me.
- Matt (Sent from mobile)
… On 16 Aug 2018, at 16:20, Riad Benguella ***@***.***> wrote:
@youknowriad commented on this pull request.
In packages/components/src/menu-item/style.scss:
> padding-left: 8px;
+
+ // Hide the keyboard shortcuts on mobile, because they aren't super-useful
+ // for most mobile users and it frees up screen space for any item
+ // with a long description.
+ display: none;
Probably fine for now, but I wonder if this should be hidden entirely in the generic MenuItem component or something specific to the Global menu item.
It can be used in the Block Settings Menu and elsewhere since it's a generic component.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Fix #9050.
Description
Hide the shortcuts for mobile screens.
How has this been tested?
Tested locally; works.
Screenshots
Types of changes
CSS change.
Checklist: