WordPress.org

Make WordPress Core

Opened 5 weeks ago

Closed 3 weeks ago

#52751 closed defect (bug) (fixed)

UI issue on Privacy Policy Guide page

Reported by: sumitsingh Owned by: davidbaumwald
Milestone: 5.7.1 Priority: normal
Severity: normal Version: 5.7
Component: Privacy Keywords: has-screenshots has-patch commit fixed-major dev-reviewed
Focuses: ui, css, administration Cc:

Description

Hi,

UI issue on Privacy Policy Guide page. See mentioned screenshot.

Thank you

Attachments (3)

Privacy ‹ Prod.png (75.3 KB) - added by sumitsingh 5 weeks ago.
button UI issue on Privacy Policy Guide tab in iPhone 5/SE
52751.diff (397 bytes) - added by audrasjb 5 weeks ago.
Privacy settings: address text overflow in copy privacy policy buttons
Capture d’écran 2021-03-12 à 11.36.49.png (101.9 KB) - added by audrasjb 5 weeks ago.
Result on mobile

Download all attachments as: .zip

Change History (25)

@sumitsingh
5 weeks ago

button UI issue on Privacy Policy Guide tab in iPhone 5/SE

#1 @SergeyBiryukov
5 weeks ago

  • Component changed from General to Privacy
  • Focuses ui css administration added
  • Keywords needs-patch added; needs-privacy-review removed
  • Milestone changed from Awaiting Review to 5.7.1

Hi there, thanks for the ticket!

Moving to 5.7.1 for investigation and testing, as this could be a result of the changes in #49264.

#2 @SergeyBiryukov
5 weeks ago

  • Keywords has-screenshots added

#3 @jaymanpandya
5 weeks ago

@sumitsingh Can you share the URL of the page where you see this bug? I cannot find where this page is!

#4 @sabernhardt
5 weeks ago

The (relative) URL is
/wp-admin/options-privacy.php?tab=policyguide

It's also here:
/wp-admin/privacy-policy-guide.php?tab=policyguide

Last edited 5 weeks ago by sabernhardt (previous) (diff)

#5 @sabernhardt
5 weeks ago

This was not usually a problem in 5.6 (depending on language) because the button had been outside the white box.

In French, it already broke at wider mobile sizes with the translation "Copier la suggestion de texte de politique de confidentialité dans le presse-papiers"

We may want something like this for mobile sizes, to wrap the text without having such a large line-height:

.wp-core-ui .privacy-text-copy {
    white-space: normal;
    padding-top: 8px;
    padding-bottom: 8px;
    line-height: 1.5;
}

We also could consider a more global style change for other buttons that might be too wide, but perhaps that would belong in another milestone/ticket.

#6 @palmiak
5 weeks ago

I agree with @sabernhardt solution, but maybe a more global variant would be better:

.wp-core-ui .button, .wp-core-ui .button-primary, .wp-core-ui .button-secondary {
white-space: normal;
line-height: 1.5;
}

There is a chance that some other buttons will have the same problem, on some languages.

EDIT:
the fix to CSS would look like this:

.wp-core-ui .button,
.wp-core-ui .button-primary,
.wp-core-ui .button-secondary {
        display: inline-block;
        text-decoration: none;
        font-size: 13px;
        line-height: 1.5; /* 28px */
        min-height: 30px;
        margin: 0;
        padding: 0 10px;
        cursor: pointer;
        border-width: 1px;
        border-style: solid;
        -webkit-appearance: none;
        border-radius: 3px;
        white-space: normal;
        box-sizing: border-box;
}

@media screen and (max-width: 782px) {

        .wp-core-ui .button,
        .wp-core-ui .button.button-large,
        .wp-core-ui .button.button-small,
        input#publish,
        input#save-post,
        a.preview {
                padding: 0 14px;
                line-height: 1.5; /* 38px */
                font-size: 14px;
                vertical-align: middle;
                min-height: 40px;
                margin-bottom: 4px;
        }

I can't see any problems with such fix right now, but maybe this nowrap and big line-height was there for a reason.

Also if we would change those - should we also remove nowrap from editor.css and jquery-ui-dialog.css

Last edited 5 weeks ago by palmiak (previous) (diff)

#7 @audrasjb
5 weeks ago

I'm a bit hesitant about introducing such a general change. I will probably introduce a lot of changes elsewhere, so I think would be really less dangerous to fix it on a case by case basis.

For this specific change, we have a dedicated class to target: .button.privacy-text-copy

I believe a general change on buttons behavior should be addressed on a dedicated ticket :)

@audrasjb
5 weeks ago

Privacy settings: address text overflow in copy privacy policy buttons

#8 follow-up: @audrasjb
5 weeks ago

  • Keywords has-patch added; needs-patch removed

One issue I can see with using white-space: normal everywhere is related to buttons displayed inside a text paragraph.

Last edited 5 weeks ago by audrasjb (previous) (diff)

#9 in reply to: ↑ 8 @palmiak
5 weeks ago

Replying to audrasjb:

One issue I can see with using white-space: normal everywhere is related to buttons displayed inside a text paragraph.

OK I see - you're right. Thanks for pointing this.

Your solution is OK IMO :) Good job.

#10 follow-up: @davidbaumwald
4 weeks ago

  • Keywords needs-design added

@audrasjb Thanks for the updated patch. It works well at resolving the overflow.

Looking at this a bit more holistically, should there be some margin between the copy and the button? Tagging for design input.

#11 in reply to: ↑ 10 @peterwilsoncc
3 weeks ago

Replying to davidbaumwald:

Looking at this a bit more holistically, should there be some margin between the copy and the button? Tagging for design input.

Testing with and without the patch the margin is unchanged but that may well be part of the bug with the button. While a specific fix is great for 5.7.1, a generic fix for 5.8 would be great so a less specific selector could be used.

This ticket was mentioned in Slack in #design by peterwilsoncc. View the logs.


3 weeks ago

#13 @audrasjb
3 weeks ago

  • Keywords needs-design removed

@davidbaumwald @peterwilsoncc yes I'd recommend to avoid any general change in a minor (but it's still nice to fix the specific issue in 5.7.1).
I believe we're good to go with this small change that fix the self-contained issue reported in this ticket :)

We should of course consider opening another ticket for 5.8 and general changes in WP-Admin buttons.

#14 @davidbaumwald
3 weeks ago

  • Owner set to davidbaumwald
  • Status changed from new to reviewing

@peterwilsoncc @audrasjb Great, thanks you both!

Picking this one up for review and commit for overflow fix.

This ticket was mentioned in Slack in #design by chaion07. View the logs.


3 weeks ago

This ticket was mentioned in Slack in #design by paaljoachim. View the logs.


3 weeks ago

#17 @paaljoachim
3 weeks ago

Looks good! Design has no comments.

#18 @peterwilsoncc
3 weeks ago

  • Keywords commit added

Thanks for the follow up folks, 52751.diff is good for commit.

#19 @peterwilsoncc
3 weeks ago

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

In 50568:

Privacy: Wrap text in buttons on privacy policy guide.

On narrow screens allow the text to wrap in the copy buttons on the privacy policy guide screen to avoid horizontal overflow of the parent container.

Props audrasjb, davidbaumwald, jaymanpandya, paaljoachim, palmiak, sabernhardt, SergeyBiryukov, sumitsingh.
Fixes #52751.

#20 @peterwilsoncc
3 weeks ago

  • Keywords fixed-major dev-feedback added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for commit to the 5.7 branch following second a committer's sign-off.

#21 @davidbaumwald
3 weeks ago

  • Keywords dev-reviewed added; dev-feedback removed

@peterwilsoncc Looks good to backport.

#22 @peterwilsoncc
3 weeks ago

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

In 50574:

Privacy: Wrap text in buttons on privacy policy guide.

On narrow screens allow the text to wrap in the copy buttons on the privacy policy guide screen to avoid horizontal overflow of the parent container.

Props audrasjb, davidbaumwald, jaymanpandya, paaljoachim, palmiak, sabernhardt, SergeyBiryukov, sumitsingh.
Merges [50568] to the 5.7 branch.
Fixes #52751.

Note: See TracTickets for help on using tickets.