Opened 4 years ago
Closed 4 years ago
#51631 closed enhancement (fixed)
Refactor Layout of the Menu screens to avoid dual-buttons and responsive issues.
Reported by: | garrett-eclipse | Owned by: | audrasjb |
---|---|---|---|
Milestone: | 5.7 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Menus | Keywords: | has-screenshots has-patch commit |
Focuses: | ui, accessibility, css, administration | Cc: |
Description
This ticket is split from #49576 to further investigate improvements on the responsive layout of the Menu screens, and hopefully avoid dual primary buttons in close proximity.
Quoting specific discussion points below.
When a menu with items nested by 2-3 levels is set, this page responsive behavior is pretty different and basically breaks for viewport widths between about 950 / 782 pixels. This is a pre-existing issue unrelated to this patch. Just to note that this page has more serious, long-standing, layout issues that would require a major refactoring. In a separate ticket :)
Why are there two Create Menu buttons next to each other like that?
slackDiscussion from @kburgoine
This was discussed during the #core-css triage meeting and the question was asked if we needed to keep both Create Menu buttons? Or if we could remove one?
Attachments (15)
Change History (36)
This ticket was mentioned in Slack in #accessibility by audrasjb. View the logs.
4 years ago
#3
@
4 years ago
- Keywords needs-patch has-screenshots added
- Milestone changed from Awaiting Review to 5.7
As per today's accessibility bug scrub, the second "Create menu" button doesn't looks necessary and can be removed.
But the redundant "Save menu" button can be useful when there is a lot of menu items.
Moving to milestone 5.7.
#4
@
4 years ago
- Keywords needs-design added
IMO I'd suggest keeping the button at the bottom as it provides a more logical form flow (actions after inputs). For the create screen definitely drop the top one.
For the Save screen am wondering if we could use JS to fix the bottom actions to always be visible but keep them in their container if you scroll past. This would allow us to also be consistent between the two views, removing the top button.
If JS isn't an option maybe we suppress the top button when there's less than X menu items. So suppressed on create and on save when say only 3 menu items available.
Thoughts? Added needs-design
to get input from design on the approach.
This ticket was mentioned in Slack in #core-css by kirstyburgoine. View the logs.
4 years ago
#6
follow-up:
↓ 11
@
4 years ago
Hello,
From my point of view, it is possible with just a little bit of css to keep the buttons visible despite the page height. I offer two patches for example with sticky positions on the header menu (51631.1.diff) or the footer menu (51631.2.diff).
These two patches use @supports to test the use of the css rule and also hide the unwanted button.
I understand that keeping the footer button is logical but I find that the sticky footer menu "hides" the rest of the menu and the rest of the page. It is less obvious that settings are at the bottom like the menu locations.
#7
@
4 years ago
- Owner set to audrasjb
- Status changed from new to reviewing
Self assigning for 5.7 consideration
#8
@
4 years ago
- Keywords needs-design removed
Thanks for the designs @maxpertici, those look like good options! I don't have an outspoken favorite, but I think the sticky bottom makes most sense as it also has the Delete option, which otherwise is kind of hard to discover.
#9
@
4 years ago
- Keywords has-patch commit added; needs-patch removed
- Status changed from reviewing to accepted
I widely tested 51631.2.diff
and it looks like a great enhancement 👍
I'd lean towards committing this proposal asap in the alpha cycle, to make sure to give some time to be widely tested. On my side the proposal looks really good (see screenshots above). Plus, it was validated by the design team.
Marking this for commit
.
#11
in reply to:
↑ 6
@
4 years ago
- Keywords commit removed
Replying to maxpertici:
I understand that keeping the footer button is logical but I find that the sticky footer menu "hides" the rest of the menu and the rest of the page. It is less obvious that settings are at the bottom like the menu locations.
I might like having a sticky footer, but I've also used this screen often and know my options well. It's probably not so friendly to new users.
Is making the footer sticky enough of a benefit? If it is sticky, the z-index
needs to be more than 10 to stay above the expanded menu items.
When navigating with the keyboard, the footer can cover the checkboxes, even when they are in focus. While the location options can be hidden in at least Chrome and Firefox, Chrome tends to scroll down to show them more often than Firefox.
Also, we avoided introducing a breakpoint of 783 pixels in wp-admin last year (ticket:49576#comment:26 and ticket:48780#comment:19). On the other hand, the block editor uses that breakpoint now.
I definitely prefer hiding/removing the top button and keeping the bottom one, especially in the case of a large menu combined with slow hosting and/or connection speed. (Saving a menu before all items load removes the unloaded items without a way to revert changes.)
#12
@
4 years ago
I propose a new patch (51631.3.diff) to take into account the remarks of @sabernhardt : the z-index and keyboard navigation.
I also added a shadow to make the footer stand out when it is above the other elements.
After thinking about new users, I tell myself that if the menu is new or short, all options will be visible.
I have no opinion to share.
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
4 years ago
#15
@
4 years ago
This ticket was discussed today during the weekly accessibility bug scrub.
The new patch needs a few more testing and possibily some testing using assistive technologies, but if everything seems fine it can be committed.
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
4 years ago
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
4 years ago
#19
@
4 years ago
After a few tries, scroll-margin-bottom introduced in patch 51631.3.diff is not a solution.
I don't know if it's possible to do without javascript to avoid keyboard navigation problems.
I am sending you a new patch where I added a little javascript to force a shift if the element with the focus is at the bottom of the page. 51631.4.diff
During last weeks #core-css triage, we discussed this issue briefly again in relation to #49576 and there was a question raised around whether we could do something purely with CSS that could fix this
https://wordpress.slack.com/archives/CQ7V4966Q/p1603398632334500
I've spent some time looking into this and can't find a way to do this purely with CSS (maybe someone else can? if so please share, I would love to know!). CSS doesn't have a way to calculate the height of a given container. You could use 100vh for the height of the viewport but, to know if the container displays entirely on the screen you would also have to know the height of the header area and the height of the container itself.
For example, if you knew the exact height of the page header area (in this case, shows the edit menu's and menu location tabs as well as the select box to choose a menu to edit) was 35vh and you also knew that the specific container for the menu items was 75vh then you could use something along the lines of a SASS function to add those two heights together and if the total is greater than 100, set a specific behaviour.
However, we can't determine any of those values because the header area height will change responsively and could have a larger height on smaller screens, where items have to stack. The size of the container also depends on the number of menu items added to it. We would need Javascript to determine those values for us, and once you start using javascript there is probably a much more elegant solution.
We could add some javascript here, it wouldn't be too difficult, but is there then a question of over-engineering?
In my mind, the simplest solution is to remove the bottom button when it is a
create menu
button and keep it there when it is asave menu
button.It's not a perfect solution because when you have first created your menu until you start adding menu items there are still two buttons in close proximity, but as you add menu items it does at least make sense as to why the bottom button is also there. Thoughts?