Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#36353 closed defect (bug) (fixed)

Hiding the Title Attribute in the screen options of Menu page not working

Reported by: sidati's profile sidati Owned by: westonruter's profile westonruter
Milestone: 4.5 Priority: normal
Severity: normal Version: 4.6
Component: Menus Keywords: has-patch commit
Focuses: ui, javascript Cc:

Description

Hi there,

On the screen options of Menus page, you cannot hide "Title Attribute" using screen options checkbox, after quick investigation, it seems the value of the field is not the same as field's class, so there is 2 ways to fix that, either change the value of the field to be title-attribute to just change the class attribute of the related p tag to be field-attr-title description description-wide

Regards,
Sidati

Attachments (4)

36353.patch (1006 bytes) - added by sidati 8 years ago.
36353.2.diff (1.0 KB) - added by westonruter 8 years ago.
36353.3.diff (3.9 KB) - added by westonruter 8 years ago.
36353.4.patch (4.7 KB) - added by sidati 8 years ago.

Download all attachments as: .zip

Change History (21)

@sidati
8 years ago

#1 @sidati
8 years ago

  • Keywords has-patch added

#2 @ocean90
8 years ago

  • Milestone changed from Awaiting Review to 4.5

Broken since [36908].

Changingtitle-attribute to attr-title in [36908] was probably wrong because it's breaking [32691] (the default setting) too.

cc: @westonruter, @afercia

@westonruter
8 years ago

#3 @westonruter
8 years ago

  • Keywords commit added

@sidati good catch. This fixes the issue. But to help ensure back-compat, I suggest we just add the class name as opposed to replacing the class name, in case any plugins are targeting it. See 36353.2.diff.

#4 @sidati
8 years ago

@westonruter good thinking :)

#5 follow-up: @ocean90
8 years ago

  • Keywords commit removed

Changingtitle-attribute to attr-title in [36908] was probably wrong because it's breaking [32691] (the default setting) too.

#6 @afercia
8 years ago

I'm afraid I won't have so much time to look at this. Seems to me either solutions would be OK, as long as the field name is the same in the Menus screen and in the Customizer, unless I'm missing something.

#7 in reply to: ↑ 5 @westonruter
8 years ago

Replying to ocean90:

Changingtitle-attribute to attr-title in [36908] was probably wrong because it's breaking [32691] (the default setting) too.

I see. So in wp_nav_menu_manage_columns() you're saying that attr-title should be returned to title-attribute in that underlying function.

@westonruter
8 years ago

#8 @westonruter
8 years ago

@sidati how about 36353.3.diff?

@sidati
8 years ago

#9 @sidati
8 years ago

@westonruter, Just apply your patch and everything works great on both screens wp-admin/nav-menus.php and the customizer, you only forgot to update the rtl stylesheet :P

#10 @westonruter
8 years ago

  • Keywords commit added

@sidati Thanks! The RTL stylesheet will be automatically updated via the build script (via grunt rtlcss). You can see that there are no RTL CSS files in the src repo.

#11 @sidati
8 years ago

@westonruter, Sorry i've been using this repo : https://core.svn.wordpress.org/trunk/.

Anyway screens options are perfect now, thank you guys for your time

#12 @westonruter
8 years ago

@ocean90 how does this look to you?

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


8 years ago

#16 @westonruter
8 years ago

  • Owner set to westonruter
  • Status changed from new to accepted

#17 @westonruter
8 years ago

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

In 37153:

Customize: Fix toggle of title attribute field visibility on nav menus admin page.

Improves alignment with nav menus in the Customizer, fixing regression introduced in [36908].

See #35273.
Props sidati, westonruter.
Fixes #36353.

Note: See TracTickets for help on using tickets.