WordPress.org

Make WordPress Core

Opened 16 months ago

Closed 16 months ago

Last modified 15 months ago

#43961 closed defect (bug) (fixed)

Privacy Policy popup covers collapsed admin menu

Reported by: littler.chicken Owned by: iandunn
Milestone: 4.9.6 Priority: normal
Severity: normal Version: 5.1
Component: Privacy Keywords: gdpr fixed-major
Focuses: Cc:

Description

On logging in to trunk this morning, saw the new Privacy Policy admin pointer pointing to the admin menu. I have my menu collapsed, and so the popup completely covers the menu icons. Assume it's a positioning issue to be tweaked in the $js_args for the pointer? If the menu is not collapsed, the positioning is okay, although it seems like it's between Tools and Settings.

The $position maybe needs to check if the menu is folded for the at position value in addition to is_rtl(). (My setup is not RTL, but that may be affected as well.)

Attachments (16)

privacy-popup.png (18.8 KB) - added by littler.chicken 16 months ago.
screen shot of privacy policy pointer blocking admin menu
43961-privacy-popup-alignment.PNG (29.0 KB) - added by ianbelanger 16 months ago.
screenshot of privacy policy popup alignment 1920px screen
43961.diff (534 bytes) - added by desrosj 16 months ago.
fix-collapsed-menu.png (163.2 KB) - added by desrosj 16 months ago.
fix-expanded-menu.png (204.1 KB) - added by desrosj 16 months ago.
fix-collapsed-rtl.png (154.5 KB) - added by desrosj 16 months ago.
fix-expanded-rtl.png (206.8 KB) - added by desrosj 16 months ago.
43961-privacy-popup-text.PNG (32.4 KB) - added by ianbelanger 16 months ago.
43961.2.diff (1.2 KB) - added by ianbelanger 16 months ago.
Patch that changes the order of the popup text to match menu items order
43961-privacy-popup-text2.PNG (28.4 KB) - added by ianbelanger 16 months ago.
Updated screenshot with 43961.diff applied
Screen Shot 2018-05-09 at 12.27.52 PM.png (130.7 KB) - added by melchoyce 16 months ago.
43961.3.diff (1.8 KB) - added by desrosj 16 months ago.
43961-privacy-popup-text3.PNG (27.8 KB) - added by ianbelanger 16 months ago.
Tested on Windows 10, Chrome, 1920 screen, looks good
43961.4.diff (1.9 KB) - added by ianbelanger 16 months ago.
merged @desrosj 43961.3.diff and reversed the Popup Headline text to Personal Data and Privacy to match order
43961-privacy-popup-text4.PNG (25.6 KB) - added by ianbelanger 16 months ago.
Updated screenshot with 43961.4.diff applied
43961-privacy-popup-text4-tablet.PNG (26.8 KB) - added by ianbelanger 16 months ago.
Tablet screenshot with 43961.4.diff applied

Download all attachments as: .zip

Change History (35)

@littler.chicken
16 months ago

screen shot of privacy policy pointer blocking admin menu

This ticket was mentioned in Slack in #gdpr-compliance by lakenh. View the logs.


16 months ago

#2 @desrosj
16 months ago

  • Milestone changed from Awaiting Review to 4.9.6

@ianbelanger
16 months ago

screenshot of privacy policy popup alignment 1920px screen

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


16 months ago

#4 @desrosj
16 months ago

  • Keywords needs-patch gdpr added

#5 @desrosj
16 months ago

Related #43996.

Edit: Changed to correct ticket number.

Last edited 16 months ago by desrosj (previous) (diff)

@desrosj
16 months ago

#6 @desrosj
16 months ago

Good catch, @littler.chicken!

43961.diff should fix the issue. It also fixes the issue described in #43996.

A few changes in 43961.diff that I am not sure are ideal:

  • The pointer is no longer pulled over the tools menu item
  • The pointer is no longer positioned in the exact middle of the Tools and Settings menu items.

The approach accounts for all RTL, screen size, and plugins adding menu item scenarios.

#7 @desrosj
16 months ago

  • Keywords has-patch 2nd-opinion added; needs-patch removed

#8 @ianbelanger
16 months ago

After seeing the patch, I think the initial placement actually made more sense. The new pages are under both Tools and Settings. Furthermore, I think that the Privacy Policy headline and paragraph should be placed under the Personal Data Export and Erasure headline and paragraph, because that's where it is located in the Admin Menu. My 2 cents

@ianbelanger
16 months ago

Patch that changes the order of the popup text to match menu items order

This ticket was mentioned in Slack in #gdpr-compliance by iandunn. View the logs.


16 months ago

#10 @melchoyce
16 months ago

Is 43961-privacy-popup-text.PNG using the default pointer spacing? (It's kind of uneven, but if it's just the way pointers are styled, I'll deal.)

@ianbelanger
16 months ago

Updated screenshot with 43961.diff applied

#11 @melchoyce
16 months ago

I have a couple recommended styling tweaks in Shot 2018-05-09 at 12.27.52 PM.png:

.wp-pointer-content h4 {
    margin: 1.33em 20px 1em;
    font-size: 1.15em;
}

.wp-pointer-content p {
    padding: 0 20px;
}

@desrosj
16 months ago

#12 @desrosj
16 months ago

43961.3.diff combines @ianbelanger's suggestion to flip the text, and @melchoyce's style changes with the bug fix.

#13 @desrosj
16 months ago

  • Keywords needs-testing added

@ianbelanger
16 months ago

Tested on Windows 10, Chrome, 1920 screen, looks good

@ianbelanger
16 months ago

merged @desrosj 43961.3.diff and reversed the Popup Headline text to Personal Data and Privacy to match order

@ianbelanger
16 months ago

Updated screenshot with 43961.4.diff applied

@ianbelanger
16 months ago

Tablet screenshot with 43961.4.diff applied

#14 @iandunn
16 months ago

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

In 43210:

Privacy: Reposition admin pointer to avoid covering collapsed menu.

Previously the pointer overlapped the menu in order to draw attention to the fact that it applies to both the Tools and Settings menus. That caused a conflict if the menu was collapsed, though, because the icons were covered by the pointer and therefore inaccessible.

Additionally, minor tweaks were made to the text order and formatting. The order of the two sections was swapped in the title and paragraph, in order to match the order of the corresponding menu items. The spacing around headings and paragraphs was tweaked to remove extraneous whitespace.

Props littler.chicken, desrosj, ianbelanger, melchoyce.
Fixes #43961.

#15 @iandunn
16 months ago

  • Keywords fixed-major added; has-patch 2nd-opinion needs-testing removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backport to 4.9.

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


16 months ago

#17 @SergeyBiryukov
16 months ago

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

In 43214:

Privacy: Reposition admin pointer to avoid covering collapsed menu.

Previously the pointer overlapped the menu in order to draw attention to the fact that it applies to both the Tools and Settings menus. That caused a conflict if the menu was collapsed, though, because the icons were covered by the pointer and therefore inaccessible.

Additionally, minor tweaks were made to the text order and formatting. The order of the two sections was swapped in the title and paragraph, in order to match the order of the corresponding menu items. The spacing around headings and paragraphs was tweaked to remove extraneous whitespace.

Props littler.chicken, desrosj, ianbelanger, melchoyce.
Merges [43210] to the 4.9 branch.
Fixes #43961.

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


16 months ago

#19 @desrosj
15 months ago

  • Component changed from Administration to Privacy

Moving to the new Privacy component.

Note: See TracTickets for help on using tickets.