WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

#17431 closed defect (bug) (fixed)

"show on screen" appears twice

Reported by: arena Owned by: nacin
Milestone: 3.4 Priority: normal
Severity: normal Version: 3.1
Component: UI Keywords: has-patch
Focuses: Cc:

Description

see images attached

Attachments (7)

Posts.jpg (18.2 KB) - added by arena 4 years ago.
Users.jpg (11.6 KB) - added by arena 4 years ago.
17431.diff (1.3 KB) - added by scribu 4 years ago.
17431.2.diff (1.2 KB) - added by dcowgill 4 years ago.
Updated diff and fixed typo.
17431.3.diff (652 bytes) - added by dcowgill 4 years ago.
Removing extra white spaces.
17431.4.diff (1.2 KB) - added by dcowgill 4 years ago.
Corrected version.
17431.5.diff (1.5 KB) - added by nacin 3 years ago.

Download all attachments as: .zip

Change History (29)

@arena4 years ago

@arena4 years ago

comment:1 @scribu4 years ago

  • Keywords needs-patch added; ui-feedback removed
  • Milestone changed from Awaiting Review to 3.2

Confirmed.

comment:2 @aaroncampbell4 years ago

  • Version changed from 3.2 to 3.1

It's like that on my 3.1 installs as well.

comment:3 @aaroncampbell4 years ago

It also affects the comments and the plugins screens. The text is the same, but it is given context for translation:

<h5><?php _ex('Show on screen', 'Metaboxes') ?></h5>
...
<h5><?php _ex('Show on screen', 'Screen Options') ?></h5>

I'm honestly not sure if this one is valid. I suppose we could come up with different text like "Number per page", but I'm not sure it's really a problem the way it is.

comment:4 @scribu4 years ago

If the text is the same, then it's not descriptive enough.

We either make them more descriptive or only show the text once.

Version 0, edited 4 years ago by scribu (next)

comment:5 follow-up: @westi4 years ago

I'm sure we have fixed a similar bug in the past.

Maybe something has returned :(

@scribu4 years ago

comment:6 @scribu4 years ago

  • Keywords has-patch added; needs-patch removed

comment:7 @arena4 years ago

Not sure 'Boxes to show' will fit for columns !!??

comment:8 @scribu4 years ago

If you take a closer look at the patch, you will see that there's a different string for columns.

comment:9 in reply to: ↑ 5 @scribu4 years ago

Replying to westi:

I'm sure we have fixed a similar bug in the past.

Maybe something has returned :(

The context was introduced in [14636], so it was never really fixed, in this sense.

comment:10 @westi4 years ago

Oh yeah now I remember.

This code is so ghetto and fragile and just needs a rewrite from scratch with a proper api so it doesn't do wierd stuff :(

comment:11 @scribu4 years ago

Sure, but until then, we can fix the simple stuff.

@dcowgill4 years ago

Updated diff and fixed typo.

comment:12 @dcowgill4 years ago

Tested and all looks good.

comment:13 @scribu4 years ago

Re 17431.2.diff: what typo?

comment:14 @dd324 years ago

Re 17431.2.diff : what typo?

Extra whitespace in the final string.

comment:15 @scribu4 years ago

Ah, finally managed to see it, on line 1793.

Note that the new patch, on line 1766, has an extra space after : and another one at EOL.

comment:16 @ryan4 years ago

  • Keywords ux-feedback added

@dcowgill4 years ago

Removing extra white spaces.

@dcowgill4 years ago

Corrected version.

comment:17 @ryan4 years ago

  • Milestone changed from 3.2 to Future Release

comment:18 @azaozz4 years ago

Best would be to gather all the output HTML in a var and if not empty output "Show on screen" followed by the rest of the HTML.

comment:19 @jane3 years ago

  • Keywords ux-feedback removed

It should only be there once per screen, but it's not really a big deal. Removing ux-feedback tag and letting the devs decide how to handle it.

comment:20 @nacin3 years ago

Given the changes to WP_Screen, these patches are stale. Here's a new one.

@nacin3 years ago

comment:21 @SergeyBiryukov3 years ago

  • Milestone changed from Future Release to 3.4

comment:22 @nacin3 years ago

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

In [19943]:

Only show 'Show on screen' once in screen options. fixes #17431.

Note: See TracTickets for help on using tickets.