WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 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 3 years ago.
Users.jpg (11.6 KB) - added by arena 3 years ago.
17431.diff (1.3 KB) - added by scribu 3 years ago.
17431.2.diff (1.2 KB) - added by dcowgill 3 years ago.
Updated diff and fixed typo.
17431.3.diff (652 bytes) - added by dcowgill 3 years ago.
Removing extra white spaces.
17431.4.diff (1.2 KB) - added by dcowgill 3 years ago.
Corrected version.
17431.5.diff (1.5 KB) - added by nacin 2 years ago.

Download all attachments as: .zip

Change History (29)

arena3 years ago

arena3 years ago

comment:1 scribu3 years ago

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

Confirmed.

comment:2 aaroncampbell3 years ago

  • Version changed from 3.2 to 3.1

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

comment:3 aaroncampbell3 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 scribu3 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 3 years ago by scribu (next)

comment:5 follow-up: westi3 years ago

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

Maybe something has returned :(

scribu3 years ago

comment:6 scribu3 years ago

  • Keywords has-patch added; needs-patch removed

comment:7 arena3 years ago

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

comment:8 scribu3 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 scribu3 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 westi3 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 scribu3 years ago

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

dcowgill3 years ago

Updated diff and fixed typo.

comment:12 dcowgill3 years ago

Tested and all looks good.

comment:13 scribu3 years ago

Re 17431.2.diff: what typo?

comment:14 dd323 years ago

Re 17431.2.diff : what typo?

Extra whitespace in the final string.

comment:15 scribu3 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 ryan3 years ago

  • Keywords ux-feedback added

dcowgill3 years ago

Removing extra white spaces.

dcowgill3 years ago

Corrected version.

comment:17 ryan3 years ago

  • Milestone changed from 3.2 to Future Release

comment:18 azaozz3 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 jane2 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 nacin2 years ago

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

nacin2 years ago

comment:21 SergeyBiryukov2 years ago

  • Milestone changed from Future Release to 3.4

comment:22 nacin2 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.