WordPress.org

Make WordPress Core

Opened 13 days ago

Closed 5 days ago

#42111 closed defect (bug) (fixed)

Heads up! modal better responsive on narrow screens

Reported by: sami.keijonen Owned by: helen
Milestone: 4.9 Priority: normal
Severity: normal Version: trunk
Component: Customize Keywords: good-first-bug has-patch needs-testing
Focuses: ui Cc:

Description

When you open Appearance >> Editor Heads up! modal dialog opens. At the moment modal goes a little bit over the content on narrow screens. See screenshot:

https://cldup.com/4vps8qPo5z.png

It could have a little CSS love.

Attachments (6)

42111.diff (43.3 KB) - added by Mirucon 13 days ago.
42111-2.diff (1.1 KB) - added by Mirucon 13 days ago.
42111-3.diff (1.4 KB) - added by Mirucon 8 days ago.
42111.png (202.8 KB) - added by Mirucon 8 days ago.
What the dialog look like
42111-2.png (216.6 KB) - added by Mirucon 8 days ago.
When menu is opened
42111.2.diff (657 bytes) - added by helen 6 days ago.

Download all attachments as: .zip

Change History (18)

#1 @sami.keijonen
13 days ago

  • Version set to trunk

#2 @melchoyce
13 days ago

  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to 4.9
  • Type changed from enhancement to defect (bug)

@Mirucon
13 days ago

#3 @Mirucon
13 days ago

  • Keywords has-patch needs-testing added; needs-patch removed

Here's a patch.

I am not sure this is a good way but at least it works.

#4 @westonruter
13 days ago

See #31779 where this modal was introduced.

Note, if you dismiss it, an easy way to get it back (and other dismissed pointers) is this WP-CLI command:

wp user meta set admin dismissed_wp_pointers ''

#5 @westonruter
13 days ago

  • Owner set to helen
  • Status changed from new to reviewing

@Mirucon it looks like there are some issues with indentation in your patch. Tabs should be used instead of spaces.

Also, there is no need to supply a patch for edit.min.css. This is generated automatically by the build process. For more on that, see https://make.wordpress.org/core/handbook/tutorials/working-with-patches/#creating-a-patch

@Mirucon
13 days ago

#6 @Mirucon
13 days ago

@westonruter Thanks, fixed!

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


9 days ago

#8 @helen
8 days ago

Hi @Mirucon - thanks for the patch. Could you possibly attach screenshots of what you see with your patch? I'm getting odd results where the modal is off the screen or not showing at all, and the toolbar at the top seems to be affected. Additionally, your patch affects other instances of .notification-dialog and associated - the post edit lock (when somebody else is already editing a post and you also head to the editor for that post) and FTP credentials ones come to mind. A solution may need to be specific to this modal since this isn't really a completely thought-out component, with enhancements noted for a later release.

To test the FTP credentials modal, you'll want to use this code snippet in a plugin file and try doing something that involves installation or upgrade, like installing a theme or plugin.

<?php
/**
 * Plugin Name: Force FTP
 * Description: Used to test FTP credentials screen.
 */

add_filter( 'filesystem_method', function() {
	return 'ftpext';
} );

add_filter( 'fs_ftp_connection_types', function() {
	return [
		'ftp'  => __( 'FTP' ),
		'ftps' => __( 'FTPS (SSL)' ),
		'ssh'  => __( 'SSH2' ),
	];
} );

@Mirucon
8 days ago

#9 @Mirucon
8 days ago

Thanks @helen, I've uploaded a new patch! It's been specified the style so that it only applies for the "Heads up!" dialog. And it now doesn't affect to toolbar.

I was not able to represent this issue - odd results where the modal is off the screen or not showing at all. Can you please make a little bit clear on this?

@Mirucon
8 days ago

What the dialog look like

@Mirucon
8 days ago

When menu is opened

@helen
6 days ago

#10 @helen
6 days ago

@Mirucon Your patch removes styling from other places that use .notification-dialog - I know it's hard to figure out what gets used where, so in the future I would suggest doing a search for usages of the class so that you test each of those instances. The toolbar problem is that it shouldn't be accessible at all, making compensation for the menu being open unnecessary, for instance.

In the interest of time, I've attached the simplest possible patch as 42111.2.diff, which also happens to fix the FTP credentials dialog on narrow screens. Some screenshots:

FTP before:

http://s.hyhs.me/n3le/image.png

FTP after:

http://s.hyhs.me/n3Rb/image.png

File editor warning after:

http://s.hyhs.me/n3SL/image.png

#11 @melchoyce
5 days ago

Looks good, going to commit 👍

#12 @melchoyce
5 days ago

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

In 41854:

Improve File Credentials / Code Editor modal responsive styles.

Makes the modal full-width and height.

Props sami.keijonen, Mirucon, helen.
Fixes #42111.

Note: See TracTickets for help on using tickets.