Make WordPress Core

Opened 3 years ago

Last modified 2 months ago

#60666 reopened defect (bug)

Unwanted section appears as a sidebar in 'Create gallery' section

Reported by: ukdrahul Owned by: joedolson
Priority: normal Milestone: Future Release
Component: Media Version: 5.0
Severity: normal Keywords: has-patch
Cc: Focuses: javascript

Description

Unwanted section with 'Actions' and 'Create gallery' texts appear in the sidebar of Media Library and nothing happens on selection or click of the texts.

To find the issue, please go through the below steps.

1.From the 'File' block add the block 'Gallery', then select Media Library.
2.Unwanted sidebar section with the specified texts appears.

Attachments (4)

Unwanted section appears for gallery block.png​ (1.4 MB ) - added by ukdrahul 3 years ago.
Unwanted sidebar appears for gallery block
60666.diff​ (908 bytes ) - added by joedolson 2 years ago.
Converts comment into patch
60666.png​ (548.4 KB ) - added by shailu25 2 years ago.
Added Attchment
media-grid.mov​ (4.0 MB ) - added by desrosj 2 years ago.

Change History (40)

@ukdrahul
3 years ago

Unwanted sidebar appears for gallery block

#1 @ababir
3 years ago

I have seen the same issue for just the image block. Here is ticket #58973. It seems like this ticket was solved on 6.5. But it still exists for the 'Image Gallery' block.

#2 @joedolson
3 years ago

  • Focuses javascript added
  • Milestone Awaiting Review → 6.6
  • Owner set to joedolson
  • Status new → accepted
  • Version → 5.0

It's appearing because it's showing the context of the gallery creation, but it is rather pointless, since it's the only option available. It doesn't do anything because there are no alternatives available.

Setting the version to 5.0 for now, I'm guessing this has existed since galleries showed up in the block editor, but I'll have to research that later.

This ticket was mentioned in ​Slack in #core-media by joedolson. ​View the logs.


3 years ago

#4 @ruchirj
3 years ago

  • Keywords changes-requested needs-testing added

Hello i just change the code and it's working for all block who have no action menu here is below,

before changes:



	_menu: function() {
		var menu = this.frame.menu,
			mode = this.get('menu'),
			actionMenuItems,
			actionMenuLength,
			view;

		if ( this.frame.menu ) {
			actionMenuItems = this.frame.menu.get('views'),
			actionMenuLength = actionMenuItems ? actionMenuItems.views.get().length : 0,
			// Show action menu only if it is active and has more than one default element.
			this.frame.$el.toggleClass( 'hide-menu', ! mode || actionMenuLength < 2 );
		}
		if ( ! mode ) {
			return;
		}

		menu.mode( mode );

		view = menu.get();
		if ( view && view.select ) {
			view.select( this.id );
		}
	},

and after changes :

	_menu: function() {
		var menu = this.frame.menu,
			mode = this.get('menu'),
			actionMenuItems,
			actionMenuLength,
			view,
            actionMenuItems = this.frame.menu.get('views'),
			actionMenuLength = actionMenuItems ? actionMenuItems.views.get().length : 0;

		if ( this.frame.menu ) {
			// Show action menu only if it is active and has more than one default element.
			this.frame.$el.toggleClass( 'hide-menu', ! mode || actionMenuLength < 2 );
		}
		if ( ! mode || actionMenuLength < 2) {
			return;
		}

		menu.mode( mode );

		view = menu.get();
		if ( view && view.select ) {
			view.select( this.id );
		}
	},

media-view.js
line 2282 to 2305.

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

#5 @ruchirj
2 years ago

hello all any one look at this please.

This ticket was mentioned in ​Slack in #core-media by antpb. ​View the logs.


2 years ago

This ticket was mentioned in ​Slack in #core by nhrrob. ​View the logs.


2 years ago

#8 @nhrrob
2 years ago

  • Keywords changes-requested removed

We have reviewed this in today's bug scrub.
Changes requested doesn't seem a valid keyword here.

Also a possible fix is given in https://core.trac.wordpress.org/ticket/60666#comment:4

Anyone wants to add a patch or PR?

@joedolson
2 years ago

Converts comment into patch

#9 follow-up: @joedolson
2 years ago

  • Keywords has-patch added

#11 @joedolson
2 years ago

I'm not sure this patch works as described; at least, not as far as I can tell. However, the original testing instructions also don't entirely make sense to me: I'm not sure what "From the 'File' block add the block 'Gallery', then select Media Library." means.

@ukdrahul Are you able to clarify the instructions for reproducing the issue? I'm not clear what the File block has to do with this.

#12 @shailu25
2 years ago

Here are the steps to reproduce the issue:

  • Create a new post/page.
  • Select the Gallery block and click on the Media Library option.
  • You will notice that the unwanted section appears in sidebar.
  • Added Attachment for Reference.
Last edited 2 years ago by shailu25 (previous) (diff)

@shailu25
2 years ago

Added Attchment

#13 @shailu25
2 years ago

Related: 58973

#14 @oglekler
2 years ago

  • Milestone 6.6 → 6.7

We have RC1 and patch needs at least testing, so, I am moving it to the next milestone.

#15 in reply to: ↑ 9 @rsiddiqe
2 years ago

Replying to joedolson:

Hey, trying to implement the patch (60666.diff) on a fresh install of 6.5 and 5.5. Can't find the file to patch (state.js) within the installation directory.

The given path is src/js/media/controllers/state.js . However searching for 'state.js' within the WordPress installation directory finds no such file.

Am I doing something wrong here?

#16 @sudipatel007
2 years ago

  • Keywords needs-testing removed

Test Report

Description
I have added patch in my local and it is working fine - https://core.trac.wordpress.org/attachment/ticket/60666/60666.diff

Environment

  • WordPress: 6.6-RC1-58573
  • PHP: 8.1.23
  • Server: nginx/1.16.0
  • Database: mysqli (Server: 8.0.16 / Client: mysqlnd 8.1.23)
  • Browser: Chrome 126.0.0.0 (macOS)
  • Theme: Twenty Nineteen 2.8

Results
Before Patch
​https://prnt.sc/PyELnbnvbjLe

After Patch
Screenshot - ​https://prnt.sc/5KT-Vx2Tch_l
Video - ​https://www.loom.com/share/71aacf85758543c386440e45254563d1?sid=6997c9ed-e284-4a97-bc4d-2042e994303e

#17 @ukdrahul
2 years ago

@joedolson As the testing went well (done by @sudipatel007 ), I was wondering if we could try to resolve this minor bug in version 6.6 and finalize it. Would that be possible?

#18 @joedolson
2 years ago

@rsiddiqe You need to apply patches to the WordPress src directory; if you don't have a state.js file, then you're looking at a copy of WordPress that's already been built.

@ukdrahul No, now that we're in the release candidate phase nothing can be committed that isn't an issue introduced during the 6.6 cycle.

#19 @dhrumilk
2 years ago

  • Keywords has-testing-info added

Test Report

Description

I have applied the https://core.trac.wordpress.org/attachment/ticket/60666/60666.diff patch locally, and it is functioning as expected.

Environment

WordPress: 6.6
PHP: 8.2.12
Server: apache
Browser: Chrome 129.0.6668.58
Theme: Twenty Twenty

Results

The patch is working as expected.

#20 @sudipatel007
2 years ago

Test Report

Description

I have added patch in my local and it is working fine - https://core.trac.wordpress.org/attachment/ticket/60666/60666.diff

Environment

  • WordPress: 6.7-alpha-59086
  • PHP: 8.2.23
  • Server: Apache/2.4.62 (Debian)
  • Database: mysqli (Server: 11.5.2-MariaDB-ubu2404 / Client: mysqlnd 8.2.23)
  • Browser: Chrome 129.0.0.0 (macOS)
  • Theme: Twenty Twenty-Four 1.2
  • MU-Plugins: None activated
  • Plugins:
    • WordPress Beta Tester 3.5.6

Results

Before Patch
​https://prnt.sc/Tgsj29KObflW

After Patch
​https://prnt.sc/mHoytwiCmvwu

#21 @joedolson
2 years ago

  • Keywords commit added

#22 @joedolson
2 years ago

  • Resolution → fixed
  • Status accepted → closed

In 59139:

Media: Hide sidebar in Create Gallery when no actions available.

Check whether the media frame menu has action items. If not, hide the sidebar. Prevents showing a sidebar that looks interactive but contains no controls that are interactive. When in Create Gallery mode, the only action available is 'Create Gallery', and it is always active.

Props ukdrahul, ababir, ruchirj, nhrrob, joedolson, shailu25, mukesh27, sudipatel007, dhrumilk.
Fixes #60666.

​@joedolson commented on ​PR #6878:


2 years ago
#23

In r59139

This ticket was mentioned in ​Slack in #core by noisysocks. ​View the logs.


2 years ago

#25 @peterwilsoncc
2 years ago

  • Resolution fixed
  • Status closed → reopened

The tests in the Gutenberg repo have picked up an error with this as the left sidebar is no longer displayed when inserting media within the classic block.

See ​https://github.com/WordPress/gutenberg/actions/runs/11116072767/job/30885672515

Reopening for follow-up.

#26 @ramonopoly
2 years ago

The tests in the Gutenberg repo have picked up an error with this as the left sidebar is no longer displayed when inserting media within the classic block.

Can confirm. Not sure what the fix is. Is it possible from within the media controller to know if it's been activated by a classic block?

#27 @joedolson
2 years ago

Yeah, this has over-reached, and it's impacting cases beyond what it should. May need to revert; but I'll explore further tomorrow.

#28 @desrosj
2 years ago

Came here because I noticed a seemingly related problem in the media library itself. Clicking on attachments in gallery view has no effect. I tried the commit prior to this change and everything seemed to work great.

#29 @joedolson
2 years ago

I'm going to revert this; while it is doing what it says it'll do, it's *also* doing a lot of other things.

I haven't been able to reproduce what you're describing, though @desrosj. Can you be specific about what environment you're using?

#30 @joedolson
2 years ago

In 59156:

Revert [59139] due to broad side effects in media library.

The change in [59139] caused all media modal menus to disappear.

Unprops joedolson.
See #60666.

#31 @desrosj
2 years ago

  • Keywords commit removed

I am using the local Docker environment. But I'm also seeing it on my personal site with beta 1 installed.

@desrosj
2 years ago

#32 @joedolson
2 years ago

  • Milestone 6.7 → 6.8

This ticket was mentioned in ​Slack in #core by audrasjb. ​View the logs.


20 months ago

#34 @audrasjb
20 months ago

  • Keywords needs-patch added; has-patch has-testing-info removed
  • Milestone 6.8 → Future Release

As per today's bug scrub: The ticket wasn't updated in 4 months, so I'm going to move it to Future Release. When a patch is ready to go, feel free to reassign it in a milestone.

This ticket was mentioned in ​PR #12122 on ​WordPress/wordpress-develop by ​@sukhendu2002.


4 months ago
#35

  • Keywords has-patch added; needs-patch removed

#36 @sukhendu2002
2 months ago

I opened PR for this ticket: ​https://github.com/WordPress/wordpress-develop/pull/12122

The previous fix in r59139 was reverted because it returned early from _menu() and prevented menu rendering in other media modal flows, including the Classic block flow.

This PR keeps the menu rendering lifecycle intact, then hides the menu only when the rendered menu has fewer than two visible actionable items. This removes the redundant Actions / Create Gallery menu in the Gallery block media modal without skipping menu setup for other flows.

Any review would be appreciated.

Note: See TracTickets for help on using tickets.