Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#52751 closed defect (bug) (fixed)

UI issue on Privacy Policy Guide page

Reported by: sumitsingh's profile sumitsingh Owned by: davidbaumwald's profile 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 (4)

Privacy ‹ Prod.png (75.3 KB) - added by sumitsingh 4 years ago.
button UI issue on Privacy Policy Guide tab in iPhone 5/SE
52751.diff (397 bytes) - added by audrasjb 4 years 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 4 years ago.
Result on mobile
Privacy ‹ Prod (1).png (65.3 KB) - added by sumitsingh 4 years ago.
Privacy Policy Guide button UI issue

Download all attachments as: .zip

Change History (27)

@sumitsingh
4 years ago

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

#1 @SergeyBiryukov
4 years 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
4 years ago

  • Keywords has-screenshots added

#3 @jaymanpandya
4 years ago

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

#4 @sabernhardt
4 years 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 4 years ago by sabernhardt (previous) (diff)

#5 @sabernhardt
4 years 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
4 years 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 4 years ago by palmiak (previous) (diff)

#7 @audrasjb
4 years 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
4 years ago

Privacy settings: address text overflow in copy privacy policy buttons

#8 follow-up: @audrasjb
4 years 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 4 years ago by audrasjb (previous) (diff)

#9 in reply to: ↑ 8 @palmiak
4 years 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 years 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
4 years 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.


4 years ago

#13 @audrasjb
4 years 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
4 years 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.


4 years ago

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


4 years ago

#17 @paaljoachim
4 years ago

Looks good! Design has no comments.

#18 @peterwilsoncc
4 years ago

  • Keywords commit added

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

#19 @peterwilsoncc
4 years 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
4 years 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
4 years ago

  • Keywords dev-reviewed added; dev-feedback removed

@peterwilsoncc Looks good to backport.

#22 @peterwilsoncc
4 years 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.

#23 @sumitsingh
4 years ago

Hi,

I have updated 5.7.1 and I think we need to some space before button?You can see mentioned screenshot.

@sumitsingh
4 years ago

Privacy Policy Guide button UI issue

Note: See TracTickets for help on using tickets.