WordPress.org

Make WordPress Core

Opened 18 months ago

Closed 18 months ago

Last modified 17 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:
PR Number:

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 18 months ago.
screen shot of privacy policy pointer blocking admin menu
43961-privacy-popup-alignment.PNG (29.0 KB) - added by ianbelanger 18 months ago.
screenshot of privacy policy popup alignment 1920px screen
43961.diff (534 bytes) - added by desrosj 18 months ago.
fix-collapsed-menu.png (163.2 KB) - added by desrosj 18 months ago.
fix-expanded-menu.png (204.1 KB) - added by desrosj 18 months ago.
fix-collapsed-rtl.png (154.5 KB) - added by desrosj 18 months ago.
fix-expanded-rtl.png (206.8 KB) - added by desrosj 18 months ago.
43961-privacy-popup-text.PNG (32.4 KB) - added by ianbelanger 18 months ago.
43961.2.diff (1.2 KB) - added by ianbelanger 18 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 18 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 18 months ago.
43961.3.diff (1.8 KB) - added by desrosj 18 months ago.
43961-privacy-popup-text3.PNG (27.8 KB) - added by ianbelanger 18 months ago.
Tested on Windows 10, Chrome, 1920 screen, looks good
43961.4.diff (1.9 KB) - added by ianbelanger 18 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 18 months ago.
Updated screenshot with 43961.4.diff applied
43961-privacy-popup-text4-tablet.PNG (26.8 KB) - added by ianbelanger 18 months ago.
Tablet screenshot with 43961.4.diff applied

Download all attachments as: .zip

Change History (35)

@littler.chicken
18 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.


18 months ago

#2 @desrosj
18 months ago

  • Milestone changed from Awaiting Review to 4.9.6

@ianbelanger
18 months ago

screenshot of privacy policy popup alignment 1920px screen

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


18 months ago

#4 @desrosj
18 months ago

  • Keywords needs-patch gdpr added

#5 @desrosj
18 months ago

Related #43996.

Edit: Changed to correct ticket number.

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

@desrosj
18 months ago

#6 @desrosj
18 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
18 months ago

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

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


18 months ago

#10 @melchoyce
18 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
18 months ago

Updated screenshot with 43961.diff applied

#11 @melchoyce
18 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
18 months ago

#12 @desrosj
18 months ago

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

#13 @desrosj
18 months ago

  • Keywords needs-testing added

@ianbelanger
18 months ago

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

@ianbelanger
18 months ago

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

@ianbelanger
18 months ago

Updated screenshot with 43961.4.diff applied

@ianbelanger
18 months ago

Tablet screenshot with 43961.4.diff applied

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


18 months ago

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


18 months ago

#19 @desrosj
17 months ago

  • Component changed from Administration to Privacy

Moving to the new Privacy component.

Note: See TracTickets for help on using tickets.