WordPress.org

Make WordPress Core

Opened 3 months ago

Closed 2 months ago

#41158 closed defect (bug) (fixed)

Increase tinymce panel z-index

Reported by: greuben Owned by: azaozz
Milestone: 4.8.1 Priority: normal
Severity: normal Version: 4.8
Component: Customize Keywords: has-patch fixed-major commit
Focuses: javascript Cc:

Description

Tinymce panels in Customizer are not visible due to low z-index value. Add forecolor button to the text widget editor toolbar to see the issue.

Attachments (4)

increase-z-index.diff (1.6 KB) - added by greuben 3 months ago.
41158.patch (1.1 KB) - added by greuben 3 months ago.
41158.2.diff (1.1 KB) - added by westonruter 3 months ago.
41158.3.patch (1.0 KB) - added by azaozz 3 months ago.

Download all attachments as: .zip

Change History (23)

#1 follow-up: @westonruter
3 months ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 4.8.1
  • Owner set to azaozz
  • Status changed from new to reviewing
  • Version changed from trunk to 4.8

@greuben There were some CSS changes in [40631] also related to z-index. Other similar CSS changes were made in Customize Posts to get the TinyMCE UI elements to appear in the right spot: https://github.com/xwp/wp-customize-posts/blob/d87ff821ddb4611a1bade7859498cbb97d2b77ac/css/customize-posts.css#L223-L240

The CSS-based change perhaps was a workaround for not being aware of the need to modify the WordPress TinyMCE plugin.

@azaozz Is this going to be the right change to make or should it be sniffing for Customizer context and only setting the value to 500001 in that case?

#2 in reply to: ↑ 1 @azaozz
3 months ago

Replying to westonruter:

...should it be sniffing for Customizer context and only setting the value to 500001 in that case?

Yeah, think we shouldn't change the z-index for all instances of the editor. Better to bump it only when used for the Text widget. Will patch it in a min.

#3 @azaozz
3 months ago

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

In 40990:

Text widget: bump the TinyMCE modals z-index to 500001 so they show in the Customizer.

Props greuben.
Fixes #41158 for trunk.

#4 @azaozz
3 months ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Committed the fix to trunk so it's easier to test.

Reopen for 4.8.1.

#5 follow-up: @westonruter
3 months ago

@azaozz I don't think we should limit it to the Text widget only. If another Customizer control uses TinyMCE, then it should likewise automatically get the fix.

#6 in reply to: ↑ 5 ; follow-up: @azaozz
3 months ago

Replying to westonruter:

Sure, what would be a "convenient" location for this in the Customizer? Then again, if the editor is used somewhere else, the z-index will be set when the Text widget initializes. It sets the z-index "globally" in tinymce, not a per-instance thing.

#7 @greuben
3 months ago

@azaozz The z-index can be moved into wp.editor.getDefaultSettings() where we can use is_customize_preview()

@greuben
3 months ago

#8 in reply to: ↑ 6 ; follow-up: @westonruter
3 months ago

Replying to azaozz:

Then again, if the editor is used somewhere else, the z-index will be set when the Text widget initializes. It sets the z-index "globally" in tinymce, not a per-instance thing.

If a Text widget is never opened, however, then this z-index logic is not called, since no Text widget control is instantiated.

Sure, what would be a "convenient" location for this in the Customizer?

Good question. I think @greuben's solution would work, but I don't think it should go inside of wp.editor.getDefaultSettings, as adding side-effects to that function doesn't seem the best. An alternative would be using wp_add_inline_script() as seen in 41158.2.diff. Or as well, that JS logic that is embedded in PHP could just as well be added to customize-controls.js like so:

  • src/wp-admin/js/customize-controls.js

     
    55075507                        } ); 
    55085508                } ()); 
    55095509 
     5510                // Make sure TinyMCE dialogs appear above Customizer UI. 
     5511                $( document ).one( "wp-before-tinymce-init", function() { 
     5512                        if ( ! tinymce.ui.FloatPanel.zIndex || tinymce.ui.FloatPanel.zIndex < 500001 ) { 
     5513                                tinymce.ui.FloatPanel.zIndex = 500001; 
     5514                        } 
     5515                } ); 
     5516 
    55105517                api.trigger( 'ready' ); 
    55115518        }); 
Last edited 3 months ago by westonruter (previous) (diff)

@westonruter
3 months ago

#9 in reply to: ↑ 8 @azaozz
3 months ago

Replying to westonruter:

I think @greuben's solution would work, but I don't think it should go inside of wp.editor.getDefaultSettings, as adding side-effects to that function doesn't seem the best.

Agreed. Best to keep it in the Customizer context.

An alternative would be using wp_add_inline_script() as seen in 41158.2.diff. Or as well, that JS logic that is embedded in PHP could just as well be added to customize-controls.js

IMHO best to do this in customize-controls.js. It is a "global" JS setting/callback not that different from blur, beforeunload, etc. :)

@azaozz
3 months ago

#10 @azaozz
3 months ago

41158.3.patch works well and is almost identical to 41158.2.diff but in customize-controls.js.

#11 follow-up: @westonruter
3 months ago

@greuben 41158.3.patch works for you as well?

#12 in reply to: ↑ 11 @greuben
3 months ago

Replying to westonruter:

@greuben 41158.3.patch works for you as well?

Yes

#13 @westonruter
3 months ago

  • Keywords commit added

#14 @azaozz
3 months ago

In 40995:

Customizer: improve setting the TinyMCE modals z-index.

Props westonruter, greuben.
See #41158.

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


2 months ago

#16 @jbpaul17
2 months ago

  • Keywords needs-testing added

#17 @westonruter
2 months ago

@jbpaul17 I don't think it needs testing. Being fixed-major it just needs to be committed to the 4.8 branch.

#18 @jbpaul17
2 months ago

  • Keywords needs-testing removed

Ok, I was going off @azaozz's comment in the bug scrub. I'm fine removing the needs-testing keyword.

#19 @westonruter
2 months ago

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

In 41068:

Widgets: Bump the TinyMCE panels' base z-index to 500001 so they show in the Customizer (such as in the Text widget).

Merges [40990] and [40995] onto 4.8 branch.
Props greuben, westonruter.
Fixes #41158 for 4.8.1.

Note: See TracTickets for help on using tickets.