WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#25373 closed enhancement (fixed)

Improve the admin-bar feature comment

Reported by: nofearinc Owned by: DrewAPicture
Milestone: 3.7 Priority: normal
Severity: normal Version:
Component: Inline Docs Keywords: has-patch commit
Focuses: Cc:

Description

Can we improve the comment for hiding the admin bar in class-wp-admin-bar.php?

Suggested update applied.

Attachments (4)

25373.patch (715 bytes) - added by nofearinc 7 years ago.
25373.2.patch (763 bytes) - added by nofearinc 7 years ago.
25373.3.patch (779 bytes) - added by nofearinc 7 years ago.
25373.4.patch (794 bytes) - added by nofearinc 7 years ago.
rename to "the Toolbar" + multiline comment

Download all attachments as: .zip

Change History (14)

@nofearinc
7 years ago

#1 @DrewAPicture
7 years ago

  • Keywords 2nd-opinion added

I believe this is actually legacy code from 3.1. As far as I know this method can't be used to disable the Toolbar any more. If that's actually the case, we should just remove the comment altogether.

#2 @nofearinc
7 years ago

That might be true actually. Either way, we could take care of the comment (or the snippet) if it's no longer in use or still needed in some way.

#3 @ocean90
7 years ago

  • Keywords needs-patch added; has-patch 2nd-opinion removed

That's not a callback for hiding the toolbar. The callback controls the padding, see _admin_bar_bump_cb() which is the default callback.
So yes, we can improve it, but with the correct description. :)

#4 follow-up: @nofearinc
7 years ago

Thanks, good call. Something like that maybe?

@nofearinc
7 years ago

#5 in reply to: ↑ 4 @ocean90
7 years ago

Replying to nofearinc:

Thanks, good call. Something like that maybe?

I would say s/apply/remove/.

@nofearinc
7 years ago

#6 @nofearinc
7 years ago

Just noticed that "default" could be "browser default" or "WordPress default" so I added extra context here. Feedback?

#7 @ocean90
7 years ago

  • Cc DrewAPicture added
  • Milestone changed from Awaiting Review to 3.7

25373.3.patch​ looks good.

@DrewAPicture Can you take a look here?

#8 @DrewAPicture
7 years ago

25373.3.patch: Looks good, just a couple of tweaks:

  • We should probably say "for the Toolbar" here for consistent messaging
  • Multi-line comments should be in block style

@nofearinc
7 years ago

rename to "the Toolbar" + multiline comment

#9 @DrewAPicture
7 years ago

  • Keywords has-patch commit added; needs-patch removed

25373.4.patch looks good.

#10 @DrewAPicture
7 years ago

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

In 25563:

Improve inline comment for removing default padding styles for the Toolbar.

Props nofearinc.
Fixes #25373.

Note: See TracTickets for help on using tickets.