Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent the admin pointer from staying below the viewport #1694

Merged
merged 1 commit into from Dec 4, 2018

Conversation

Projects
None yet
2 participants
@kienstra
Copy link
Collaborator

kienstra commented Dec 4, 2018

Steps to reproduce

  1. Add several admin menu items above the AMP plugin's menu item:

extra-test-menu-items

This snippet might help:

/**
 * Adds 10 test menu items above and below the AMP menu.
 */
add_action( 'admin_menu', function() {
	$upper_menu_name = __( 'Test menu a', 'amp-scratchpad' );
	$lower_menu_name = __( 'Test menu b', 'amp-scratchpad' );

	for ( $i = 0; $i < 10; $i++ ) {
		add_menu_page( $upper_menu_name, $upper_menu_name, 'edit_posts', $upper_menu_name, '', '', $i );
		add_menu_page( $lower_menu_name, $lower_menu_name, 'edit_posts', $lower_menu_name, '', '', 100 + $i );
	}
} );
  1. Expected: AMP admin pointer is visible on scrolling down to the AMP menu

  2. Actual: the admin pointer is not visible:

admin-pointer-scrolling-down

This PR simply removes the position: fixed rule that caused the pointer to be hidden. But that has a side effect.

The position: fixed style caused the admin pointer to always point to the AMP menu item. But now it doesn't.

It looks like that's also the case with the privacy admin pointer from WordPress 4.9.6. Notice how the privacy admin pointer doesn't always point to the Tools.

Privacy Admin Pointer

admin-pointer-10

AMP Plugin Admin Pointer With This PR

admin-pointer-privacy

There might be a way to have the admin pointer still point to the right item, but not be hidden. I couldn't find it, but CSS isn't my strong suit 馃槃

Fixes #1689

Prevent the admin pointer from staying below the viewport
When there are a lot of admin menu items,
the admin pointer can stay below the viewport,
no matter how much you scroll.
This looks to be because of the position: fixed styling.
So simply remove that.

@kienstra kienstra changed the base branch from develop to 1.0 Dec 4, 2018

@kienstra

This comment has been minimized.

Copy link
Collaborator Author

kienstra commented Dec 4, 2018

Request For Review

Hi @westonruter,
Could you please review this PR for #1689?

Thanks 馃槃

@kienstra kienstra requested a review from westonruter Dec 4, 2018

@westonruter westonruter added this to the v1.0 milestone Dec 4, 2018

@westonruter westonruter merged commit 7330c59 into 1.0 Dec 4, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@westonruter westonruter deleted the fix/1689-admin-pointer branch Dec 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.