WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#26630 closed defect (bug) (fixed)

Plugin Dashicons in admin menu have a glitch in IE

Reported by: TobiasBg Owned by: ocean90
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.8
Component: Administration Keywords: has-patch commit
Focuses: ui Cc:

Description (last modified by TobiasBg)

I'm using a Dashicon icon in a plugin's admin menu entry:

add_menu_page( 'Plugin', $entry_name, $min_access_cap, 'slug', $callback, 'dashicons-list-view', $position );

Note the 'dashicons-list-view' as the $icon_url parameter, which let's the admin menu entry use the List View Dashicon since [26664].

This works in all browsers, except in IE (tested with IE 11 on different computers and different sites), where there's a small "residue" of some control character icon on the lower right corner of the actual icon:
https://core.trac.wordpress.org/raw-attachment/ticket/26630/ie-dashicons-bug.png

When investigating this with the developer tools in IE, this disappears once I remove the dashicons class from the div and leave just the dashicons-list-view class.

Attachments (9)

ie-dashicons-bug.png (4.2 KB) - added by TobiasBg 3 years ago.
Small glitch on admin menu Dashicons in IE
26630.patch (766 bytes) - added by ocean90 3 years ago.
26630.2.patch (1.3 KB) - added by ocean90 3 years ago.
26630.3.patch (20.5 KB) - added by ocean90 2 years ago.
.dashicons-before
26630.4.patch (749 bytes) - added by ocean90 2 years ago.
26630.5.patch (534 bytes) - added by ocean90 2 years ago.
26630.6.patch (684 bytes) - added by ocean90 2 years ago.
26630.7.patch (548 bytes) - added by SergeyBiryukov 2 years ago.
26630.8.patch (462 bytes) - added by ocean90 2 years ago.

Download all attachments as: .zip

Change History (29)

@TobiasBg
3 years ago

Small glitch on admin menu Dashicons in IE

#1 @SergeyBiryukov
3 years ago

  • Keywords ui-focus added

#2 follow-up: @TobiasBg
3 years ago

  • Description modified (diff)

I received a report for this happening on 3.8.1 with IE 8 as well.
Can somebody confirm this for any IE on trunk?

Quick test code (e.g. for functions.php):

function add_my_plugin_icon_to_menu() {
  add_menu_page( 'My Plugin', 'My Plugin', 'read', 'my_plugin', '__return_true', 'dashicons-list-view' );
}
add_action( 'admin_menu', 'add_my_plugin_icon_to_menu' );

#3 in reply to: ↑ 2 @ocean90
3 years ago

Replying to TobiasBg:

Can somebody confirm this for any IE on trunk?

Tested in IE9 and can confirm. Seems like it has something to do with the <br> element, when removing the inherited styles of the element the *whatever* goes away.

@ocean90
3 years ago

@ocean90
3 years ago

#4 @ocean90
3 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 3.9

26630.2.patch is the way we should go.

The .dashicon class is pretty useless here. We have the styles already in div.wp-menu-image:before. I also noticed that the Dashicon height was defined as 36px and width as 20px - a Dashicon is build for grid of 20x20px which shouldn't be changed, at least not different from each side. A side effect of this was, that the images aren't vertically centered. A Dashicon's height is now 20px. With 7px padding for top and bottom we're in line with an item height of 34px. This also affects SVG icons.

My testcode:

function add_my_plugin_icon_to_menu() {
	add_menu_page( 'My Plugin', 'My Plugin', 'read', 'my_plugin', '__return_true', 'dashicons-list-view' );

	$svg = 'data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0idXRmLTgiPz4NCjwhLS0gR2VuZXJhdG9yOiBBZG9iZSBJbGx1c3RyYXRvciAxNS4wLjAsIFNWRyBFeHBvcnQgUGx1Zy1JbiAuIFNWRyBWZXJzaW9uOiA2LjAwIEJ1aWxkIDApICAtLT4NCjwhRE9DVFlQRSBzdmcgUFVCTElDICItLy9XM0MvL0RURCBTVkcgMS4xLy9FTiIgImh0dHA6Ly93d3cudzMub3JnL0dyYXBoaWNzL1NWRy8xLjEvRFREL3N2ZzExLmR0ZCI+DQo8c3ZnIHZlcnNpb249IjEuMSIgaWQ9IkxheWVyXzEiIHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyIgeG1sbnM6eGxpbms9Imh0dHA6Ly93d3cudzMub3JnLzE5OTkveGxpbmsiIHg9IjBweCIgeT0iMHB4Ig0KCSB3aWR0aD0iMzU1LjAyNHB4IiBoZWlnaHQ9IjQzNS4wNnB4IiB2aWV3Qm94PSIwIDAgMzU1LjAyNCA0MzUuMDYiIGVuYWJsZS1iYWNrZ3JvdW5kPSJuZXcgMCAwIDM1NS4wMjQgNDM1LjA2Ig0KCSB4bWw6c3BhY2U9InByZXNlcnZlIj4NCjxnPg0KCTxnPg0KCQk8cGF0aCBmaWxsPSIjRkZGRkZGIiBkPSJNMTc5LjIzNywwQzEzOC45ODIsMCw4My4zODgsMTEuNTAyLDYwLjM4NCwyMS4wODdzLTM4LjM0LDE5LjE3LTQzLjEzMiw0My4xMzJMMCwxOTcuMDZ2MTgzLjA0NWgyOS43MTINCgkJCXYyOC43MDVjMCwzNSw1MS4yMTgsMzUsNTEuMjE4LDB2LTI4LjcwNWg5NC41NTdoMC4zaDk4LjMwN3YyOC43MDVjMCwzNSw1MS4yMTgsMzUsNTEuMjE4LDB2LTI4LjcwNWgyOS43MTJWMTk3LjA2TDMzNy43NzIsNjQuMjE5DQoJCQljLTQuNzkyLTIzLjk2Mi0yMC4xMjgtMzMuNTQ3LTQzLjEzMi00My4xMzJDMjcxLjYzNiwxMS41MDIsMjE2LjA0MiwwLDE3NS43ODcsMCIvPg0KCTwvZz4NCgk8Zz4NCgkJPGc+DQoJCQk8Zz4NCgkJCQk8Y2lyY2xlIGN4PSIyOTkuNjgyIiBjeT0iMjk4LjE1MyIgcj0iMjQuNTciLz4NCgkJCQk8cGF0aCBkPSJNMjk5LjY4MiwyOTguMTUzIi8+DQoJCQk8L2c+DQoJCQk8Zz4NCgkJCQk8Y2lyY2xlIGN4PSI1NS4zNDIiIGN5PSIyOTguMTUzIiByPSIyNC41NyIvPg0KCQkJCTxwYXRoIGQ9Ik01NS4zNDIsMjk4LjE1MyIvPg0KCQkJPC9nPg0KCQk8L2c+DQoJCTxnPg0KCQkJPHBhdGggZD0iTTE3NS42MTIsNTUuMTEzaC03MS4xMzZjLTE0LjM3OCwwLTE0LjM3OC0yMS41NjYsMC0yMS41NjZoNzEuMzExaDc0Ljc2MWMxNC4zNzgsMCwxNC4zNzgsMjEuNTY2LDAsMjEuNTY2SDE3NS42MTJ6Ii8+DQoJCTwvZz4NCgkJPGc+DQoJCQk8cGF0aCBkPSJNMTc1LjYxMiw3Ni44ODdINjUuNzE1Yy0xNS4xODgsMC0xOS4xNTYsNy43MTctMjEsMTkuNDIzbC0xMy40MjgsOTYuMzQ4Yy0xLjI1Miw5LjIzNSwxLjQxOSwxOC40MDIsMTQuMTc4LDE4LjQwMg0KCQkJCWgxMzAuMzIyaDEzMy43NzJjMTIuNzU5LDAsMTUuNDMtOS4xNjcsMTQuMTc4LTE4LjQwMkwzMTAuMzA5LDk2LjMxYy0xLjg0NC0xMS43MDYtNS44MTItMTkuNDIzLTIxLTE5LjQyM0gxNzUuNjEyeiIvPg0KCQk8L2c+DQoJPC9nPg0KPC9nPg0KPC9zdmc+DQo=';
	add_menu_page( 'My Plugin 2', 'My Plugin 2', 'read', 'my_plugin_2', '__return_true', $svg );

	$image = 'https://make.wordpress.org/core/wp-content/plugins/jetpack/modules/contact-form//images/grunion-menu.png';
	add_menu_page( 'My Plugin 3', 'My Plugin 3', 'read', 'my_plugin_3', '__return_true', $image );

	add_menu_page( 'My Plugin 4', 'My Plugin 4', 'read', 'my_plugin_4', '__return_true', 'none' );
}
add_action( 'admin_menu', 'add_my_plugin_icon_to_menu' );

#5 @ocean90
2 years ago

In 27410:

Admin Menu: Center menu images vertically, see #26630.

@ocean90
2 years ago

.dashicons-before

#6 @ocean90
2 years ago

  • Owner set to ocean90
  • Resolution set to fixed
  • Status changed from new to closed

In 27418:

Introduce a .dashicons-before CSS class.

If you want to use a Dashicon before an element you can use the class because it will not change the elements content styling. With that you don't need to copy the entire .dashicons styling to your :before styling anymore.
To demonstrate this change, Admin Menu now uses Dashicons classes directly.

And it fixes a glitch in IE.

fixes #26630.

This ticket was mentioned in IRC in #wordpress-dev by kovshenin. View the logs.


2 years ago

@ocean90
2 years ago

@ocean90
2 years ago

@ocean90
2 years ago

#8 @kovshenin
2 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#9 @ocean90
2 years ago

In 27425:

Admin Menu: Use .dashicons-before as default $img_class.

props kovshenin for noticing and testing.
see #26630.

#10 @kovshenin
2 years ago

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

This ticket was mentioned in IRC in #wordpress-dev by ipstenu. View the logs.


2 years ago

#12 @ocean90
2 years ago

In 27444:

Add missing Dashicons classes to Network Admin menu, see #26630.

#13 @jdgrimes
2 years ago

  • Keywords has-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Now menus being registered without specifying an image don't get the generic gear image that they used to. They are just blank. That wasn't intended, was it?

This used to show the gear icon:

add_menu_page( 'My Plugin 4', 'My Plugin 4', 'read', 'my_plugin_4', '__return_true' );

Now it doesn't show any image at all.

#14 @SergeyBiryukov
2 years ago

  • Keywords has-patch commit added

#15 @ocean90
2 years ago

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

@Sergej Your patch isn't correct because it sets also an icon if none is specified, see https://cloudup.com/iAOrjMaL0Qx.

New test code:

function add_my_plugin_icon_to_menu() {
	add_menu_page( 'My Plugin', 'My Plugin', 'read', 'my_plugin', '__return_true', 'dashicons-list-view' );

	$svg = 'data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0idXRmLTgiPz4NCjwhLS0gR2VuZXJhdG9yOiBBZG9iZSBJbGx1c3RyYXRvciAxNS4wLjAsIFNWRyBFeHBvcnQgUGx1Zy1JbiAuIFNWRyBWZXJzaW9uOiA2LjAwIEJ1aWxkIDApICAtLT4NCjwhRE9DVFlQRSBzdmcgUFVCTElDICItLy9XM0MvL0RURCBTVkcgMS4xLy9FTiIgImh0dHA6Ly93d3cudzMub3JnL0dyYXBoaWNzL1NWRy8xLjEvRFREL3N2ZzExLmR0ZCI+DQo8c3ZnIHZlcnNpb249IjEuMSIgaWQ9IkxheWVyXzEiIHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyIgeG1sbnM6eGxpbms9Imh0dHA6Ly93d3cudzMub3JnLzE5OTkveGxpbmsiIHg9IjBweCIgeT0iMHB4Ig0KCSB3aWR0aD0iMzU1LjAyNHB4IiBoZWlnaHQ9IjQzNS4wNnB4IiB2aWV3Qm94PSIwIDAgMzU1LjAyNCA0MzUuMDYiIGVuYWJsZS1iYWNrZ3JvdW5kPSJuZXcgMCAwIDM1NS4wMjQgNDM1LjA2Ig0KCSB4bWw6c3BhY2U9InByZXNlcnZlIj4NCjxnPg0KCTxnPg0KCQk8cGF0aCBmaWxsPSIjRkZGRkZGIiBkPSJNMTc5LjIzNywwQzEzOC45ODIsMCw4My4zODgsMTEuNTAyLDYwLjM4NCwyMS4wODdzLTM4LjM0LDE5LjE3LTQzLjEzMiw0My4xMzJMMCwxOTcuMDZ2MTgzLjA0NWgyOS43MTINCgkJCXYyOC43MDVjMCwzNSw1MS4yMTgsMzUsNTEuMjE4LDB2LTI4LjcwNWg5NC41NTdoMC4zaDk4LjMwN3YyOC43MDVjMCwzNSw1MS4yMTgsMzUsNTEuMjE4LDB2LTI4LjcwNWgyOS43MTJWMTk3LjA2TDMzNy43NzIsNjQuMjE5DQoJCQljLTQuNzkyLTIzLjk2Mi0yMC4xMjgtMzMuNTQ3LTQzLjEzMi00My4xMzJDMjcxLjYzNiwxMS41MDIsMjE2LjA0MiwwLDE3NS43ODcsMCIvPg0KCTwvZz4NCgk8Zz4NCgkJPGc+DQoJCQk8Zz4NCgkJCQk8Y2lyY2xlIGN4PSIyOTkuNjgyIiBjeT0iMjk4LjE1MyIgcj0iMjQuNTciLz4NCgkJCQk8cGF0aCBkPSJNMjk5LjY4MiwyOTguMTUzIi8+DQoJCQk8L2c+DQoJCQk8Zz4NCgkJCQk8Y2lyY2xlIGN4PSI1NS4zNDIiIGN5PSIyOTguMTUzIiByPSIyNC41NyIvPg0KCQkJCTxwYXRoIGQ9Ik01NS4zNDIsMjk4LjE1MyIvPg0KCQkJPC9nPg0KCQk8L2c+DQoJCTxnPg0KCQkJPHBhdGggZD0iTTE3NS42MTIsNTUuMTEzaC03MS4xMzZjLTE0LjM3OCwwLTE0LjM3OC0yMS41NjYsMC0yMS41NjZoNzEuMzExaDc0Ljc2MWMxNC4zNzgsMCwxNC4zNzgsMjEuNTY2LDAsMjEuNTY2SDE3NS42MTJ6Ii8+DQoJCTwvZz4NCgkJPGc+DQoJCQk8cGF0aCBkPSJNMTc1LjYxMiw3Ni44ODdINjUuNzE1Yy0xNS4xODgsMC0xOS4xNTYsNy43MTctMjEsMTkuNDIzbC0xMy40MjgsOTYuMzQ4Yy0xLjI1Miw5LjIzNSwxLjQxOSwxOC40MDIsMTQuMTc4LDE4LjQwMg0KCQkJCWgxMzAuMzIyaDEzMy43NzJjMTIuNzU5LDAsMTUuNDMtOS4xNjcsMTQuMTc4LTE4LjQwMkwzMTAuMzA5LDk2LjMxYy0xLjg0NC0xMS43MDYtNS44MTItMTkuNDIzLTIxLTE5LjQyM0gxNzUuNjEyeiIvPg0KCQk8L2c+DQoJPC9nPg0KPC9nPg0KPC9zdmc+DQo=';
	add_menu_page( 'My Plugin 2', 'My Plugin 2', 'read', 'my_plugin_2', '__return_true', $svg );

	$image = 'https://make.wordpress.org/core/wp-content/plugins/jetpack/modules/contact-form//images/grunion-menu.png';
	add_menu_page( 'My Plugin 3', 'My Plugin 3', 'read', 'my_plugin_3', '__return_true', $image );

	add_menu_page( 'My Plugin 4', 'My Plugin 4', 'read', 'my_plugin_4', '__return_true', 'none' );

	add_menu_page( 'My Plugin 5', 'My Plugin 5', 'read', 'my_plugin_5', '__return_true' );
}
add_action( 'admin_menu', 'add_my_plugin_icon_to_menu' );

3.8.1: https://cloudup.com/cgr4tOVKlMr
Trunk: https://cloudup.com/c9JSCc5DNZo

@ocean90
2 years ago

#16 @ocean90
2 years ago

  • Keywords has-patch added; needs-patch removed

26630.8.patch updates the default value for $icon_url in add_menu_page().

#17 @SergeyBiryukov
2 years ago

  • Keywords commit added

26630.8.patch looks good.

#18 @ocean90
2 years ago

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

In 27482:

Admin Menu: Set default value of $icon_url in add_menu_page() to dashicons-admin-generic, fixes #26630.

#19 @ocean90
2 years ago

In 28284:

Add missing Dashicons classes to User Admin menu.

props imath.
see #28144, #26630.

#20 @nacin
2 years ago

In 28344:

Add missing Dashicons classes to User Admin menu.

Merges [28284] to the 3.9 branch.

props imath.
fixes #28144, #26630.

Note: See TracTickets for help on using tickets.