WordPress.org

Make WordPress Core

Opened 3 weeks ago

Closed 13 days ago

Last modified 7 days 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: trunk
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 3 weeks ago.
screen shot of privacy policy pointer blocking admin menu
43961-privacy-popup-alignment.PNG (29.0 KB) - added by ianbelanger 3 weeks ago.
screenshot of privacy policy popup alignment 1920px screen
43961.diff (534 bytes) - added by desrosj 2 weeks ago.
fix-collapsed-menu.png (163.2 KB) - added by desrosj 2 weeks ago.
fix-expanded-menu.png (204.1 KB) - added by desrosj 2 weeks ago.
fix-collapsed-rtl.png (154.5 KB) - added by desrosj 2 weeks ago.
fix-expanded-rtl.png (206.8 KB) - added by desrosj 2 weeks ago.
43961-privacy-popup-text.PNG (32.4 KB) - added by ianbelanger 2 weeks ago.
43961.2.diff (1.2 KB) - added by ianbelanger 2 weeks 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 2 weeks ago.
Updated screenshot with 43961.diff applied
Screen Shot 2018-05-09 at 12.27.52 PM.png (130.7 KB) - added by melchoyce 2 weeks ago.
43961.3.diff (1.8 KB) - added by desrosj 2 weeks ago.
43961-privacy-popup-text3.PNG (27.8 KB) - added by ianbelanger 2 weeks ago.
Tested on Windows 10, Chrome, 1920 screen, looks good
43961.4.diff (1.9 KB) - added by ianbelanger 2 weeks 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 2 weeks ago.
Updated screenshot with 43961.4.diff applied
43961-privacy-popup-text4-tablet.PNG (26.8 KB) - added by ianbelanger 2 weeks ago.
Tablet screenshot with 43961.4.diff applied

Download all attachments as: .zip

Change History (35)

@littler.chicken
3 weeks ago

screen shot of privacy policy pointer blocking admin menu

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


3 weeks ago

#2 @desrosj
3 weeks ago

  • Milestone changed from Awaiting Review to 4.9.6

@ianbelanger
3 weeks ago

screenshot of privacy policy popup alignment 1920px screen

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


2 weeks ago

#4 @desrosj
2 weeks ago

  • Keywords needs-patch gdpr added

#5 @desrosj
2 weeks ago

Related #43996.

Edit: Changed to correct ticket number.

Last edited 2 weeks ago by desrosj (previous) (diff)

@desrosj
2 weeks ago

#6 @desrosj
2 weeks 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
2 weeks ago

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

#8 @ianbelanger
2 weeks 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
2 weeks 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.


2 weeks ago

#10 @melchoyce
2 weeks 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
2 weeks ago

Updated screenshot with 43961.diff applied

#11 @melchoyce
2 weeks 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
2 weeks ago

#12 @desrosj
2 weeks ago

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

#13 @desrosj
2 weeks ago

  • Keywords needs-testing added

@ianbelanger
2 weeks ago

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

@ianbelanger
2 weeks ago

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

@ianbelanger
2 weeks ago

Updated screenshot with 43961.4.diff applied

@ianbelanger
2 weeks ago

Tablet screenshot with 43961.4.diff applied

#14 @iandunn
2 weeks 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
2 weeks 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.


13 days ago

#17 @SergeyBiryukov
13 days 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.


13 days ago

#19 @desrosj
7 days ago

  • Component changed from Administration to Privacy

Moving to the new Privacy component.

Note: See TracTickets for help on using tickets.