Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#54799 closed defect (bug) (fixed)

Active buttons Bulk select are confusing while menu is empty

Reported by: oglekler's profile oglekler Owned by: audrasjb's profile audrasjb
Milestone: 6.1 Priority: normal
Severity: normal Version: 5.8
Component: Menus Keywords: has-patch has-testing-info commit needs-user-docs
Focuses: ui, administration Cc:

Description

Even if we are going to FSE, legacy menu will be with us a long time and this interface is very difficult to users. Active buttons which not doing anything making things less clear.

Attachments (7)

2022-01-12_13-58-18.png (33.2 KB) - added by oglekler 3 years ago.
54799.patch (1.2 KB) - added by krishaweb 3 years ago.
Here is the patch to remove the bulk action button when a menu item is empty.
54799.hide_style.diff (1.3 KB) - added by costdev 3 years ago.
Alternative approach that uses the existing $hide_style variable which is set when isset( $menu_items ) && 0 === count( $menu_items ).
54799.2.patch (2.0 KB) - added by krishaweb 2 years ago.
Updated patch
before-patch-menu-creation.gif (466.9 KB) - added by audrasjb 2 years ago.
Before patch
after-patch-menu-creation.gif (374.9 KB) - added by audrasjb 2 years ago.
After patch: works fine with menu creation
after-patch-menu-items-removal.gif (751.2 KB) - added by audrasjb 2 years ago.
After patch: works fine with menu items removal

Download all attachments as: .zip

Change History (22)

#1 @SergeyBiryukov
3 years ago

  • Component changed from General to Menus
  • Focuses ui administration added

#2 @SergeyBiryukov
3 years ago

  • Version set to 5.8

Introduced in [51006] / #21603, see also [51539] / #53654.

@krishaweb
3 years ago

Here is the patch to remove the bulk action button when a menu item is empty.

#3 @krishaweb
3 years ago

  • Keywords has-patch added

@costdev
3 years ago

Alternative approach that uses the existing $hide_style variable which is set when isset( $menu_items ) && 0 === count( $menu_items ).

#4 @SergeyBiryukov
3 years ago

  • Milestone changed from Awaiting Review to 6.0

#5 @costdev
3 years ago

  • Keywords needs-testing has-testing-info added

Testing Info

Steps to reproduce

  1. Ensure that a classic theme is active.
  2. Navigate to Appearance > Menus.
  3. Create a new menu, name and save it.
  4. Two "Bulk Select" check buttons should appear.

Steps to test 54799.patch

  1. Apply 54799.patch.
  2. Refresh the menu page.
  3. No "Bulk Select" check buttons should appear.
  4. Add a menu item.
  5. Two "Bulk Select" check buttons should appear.
  6. Delete the menu item.
  7. No "Bulk Select" check buttons should appear.
  8. Revert the patch.

Steps to test 54799.hide_style.diff

  1. Apply 54799.hide_style.diff.
  2. Repeat steps 2-8 of the previous section.

Report

A test report template is available here (screencast optional).

Please report whether each patch is successful.


If both patches are successful, it is a matter of choosing which implementation to consider for commit.

Last edited 3 years ago by costdev (previous) (diff)

#6 @Boniu91
2 years ago

@costdev It looks like the changes are applied only after saving the menu. Here are two unexpected scenarios:

  1. Create a new menu
  2. Add two pages to the menu (without saving it)
  3. Elements are added to menu, bulk options are not there
  1. Create a new menu
  2. Add two pages to the menu and save it
  3. Remove all pages from the menu (without saving it)
  4. The menu is empty, bulk options are still present

Is it something that should be covered?

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


2 years ago

#8 @costdev
2 years ago

@Boniu91 Thanks for testing! These should be covered, and I'll try to get the patch refreshed soon.

#9 @costdev
2 years ago

  • Keywords changes-requested added
  • Milestone changed from 6.0 to 6.1

Unfortunately, I didn't get time to work on updating the patch. With 6.0 RC1 tomorrow, I'm moving this to the 6.1 milestone.

@krishaweb
2 years ago

Updated patch

#10 @krishaweb
2 years ago

  • Keywords changes-requested removed

Hey @costdev @Boniu91,
I have update the patch according to your suggestion, Please check with the latest patch.

Thanks

Last edited 2 years ago by krishaweb (previous) (diff)

#11 @hugodevos
2 years ago

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: REPLACE_WITH_PATCH_URL

Environment

  • OS: macOS 12.3.1
  • Web Server: Nginx
  • PHP: 7.4.29
  • WordPress: 6.1-alpha-53344-src
  • Browser: Chrome 105.0.5195.102 (Official Build) (x86_64)
  • Theme: Twenty Twenty-Two (and Twenty Twenty-One)
  • Active Plugins:

Actual Results

  • With the patch installed it works mostly as expected.
  • After installing the patch, the effect is not on reloading the page as mentioned in the Testing info in #5. It only disappears after saving the menu like mentioned in #6.
  • Any newly created menu after installing the patch works as described.

@audrasjb
2 years ago

Before patch

@audrasjb
2 years ago

After patch: works fine with menu creation

@audrasjb
2 years ago

After patch: works fine with menu items removal

#12 @audrasjb
2 years ago

I tested several use case and it looks like we're good to go.
Self assigning for commit.

#13 @audrasjb
2 years ago

  • Keywords commit added; needs-testing removed
  • Owner set to audrasjb
  • Status changed from new to accepted

#14 @audrasjb
2 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 54316:

Menus: Remove bulk action checkboxes when menu is empty.

This changeset removes the bulk action checkboxes when there is no menu item to select.

Follow-up to [51006], [51539].

Props oglekler, krishaweb, costdev, Boniu91, hugodevos, audrasjb.
Fixes #54799.
See #21603, #53654.

#15 @milana_cap
2 years ago

  • Keywords needs-user-docs added

Might need just updating screenshots in HelpHub.

Note: See TracTickets for help on using tickets.