WordPress.org

Make WordPress Core

Opened 3 months ago

Closed 8 weeks ago

Last modified 7 weeks ago

#48550 closed feature request (fixed)

Twenty Twenty: Customizer option to show or hide author bio

Reported by: williampatton Owned by: ianbelanger
Milestone: 5.3.1 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch commit
Focuses: ui Cc:
PR Number:

Description

Consider adding an option to show/hide the author bio.

A PR exists with code for this one at the original repo which a patch can be generated from.

Issue: https://github.com/WordPress/twentytwenty/issues/524
PR: https://github.com/WordPress/twentytwenty/pull/525

CC: @acosmin

Attachments (3)

48550.patch (2.0 KB) - added by nielslange 2 months ago.
#48550-show-and-hide-author-bio.png (1.9 MB) - added by nielslange 2 months ago.
48550-1.diff (2.0 KB) - added by nielslange 2 months ago.

Download all attachments as: .zip

Change History (22)

#1 @williampatton
3 months ago

  • Type changed from defect (bug) to feature request

#2 @ianbelanger
3 months ago

  • Keywords needs-patch added

@nielslange
2 months ago

#3 @nielslange
2 months ago

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

@williampatton & @ianbelanger I challenged myself to create a patch without looking at @acosmin's proposed solution and the result is pretty close. Feel free to test it and give props to @acosmin for fixing this issue first. 😀

Last edited 2 months ago by nielslange (previous) (diff)

#4 @acosmin
2 months ago

@nielslange looks ok to me :)

#5 @nielslange
2 months ago

@ianbelanger This could technically be shipped with 5.3.1, right?

#6 @nielslange
2 months ago

  • Owner set to nielslange
  • Status changed from new to assigned

#7 @ianbelanger
2 months ago

  • Milestone changed from Awaiting Review to 5.3.1

#8 follow-up: @dlh
2 months ago

Do note that this patch will cause the author bio to be hidden on existing installations until someone goes into the Customizer and saves the default setting, I believe.

@nielslange
2 months ago

#9 in reply to: ↑ 8 @nielslange
2 months ago

Replying to dlh:

Do note that this patch will cause the author bio to be hidden on existing installations until someone goes into the Customizer and saves the default setting, I believe.

Good catch, David! I just added a default statement to get_theme_mod( 'show_author_bio', true ), which should do the trick. Would you mind running a quick test, to see if this adjustment fixes the issue you've mentioned?

#10 follow-up: @dlh
2 months ago

That should do it, @nielslange!

It would be nice if the default value was centralized into a variable or constant and documented so that the separate uses with get_theme_mod() and WP_Customize_Setting stayed in sync. But I'm not sure where it would be best to try to do that in Twenty Twenty.

#11 in reply to: ↑ 10 @nielslange
2 months ago

  • Focuses ui added

It would be nice if the default value was centralized into a variable or constant and documented so that the separate uses with get_theme_mod() and WP_Customize_Setting stayed in sync. But I'm not sure where it would be best to try to do that in Twenty Twenty.

Isn't show_author_bio the centralized "variable" in this case, @dlh? I'm not sure I understand your question correctly here. 🤔

#12 @dlh
2 months ago

Sorry, I mean the value of true now being passed both as the $default to get_theme_mod() and the 'default' to WP_Customize_Setting. It would be easy for a future developer to change one in a patch without realizing the other should be changed to match, assuming that it's not just coincidence that they're the same now.

#13 @ianbelanger
8 weeks ago

  • Keywords commit added; needs-testing removed

This looks good, I tested the feature and it works as intended. I am going to mark it for commit, but I am not sure that we can get it in on a minor release. If we can, I will get t committed.

Thanks for the patch @nielslange and for testing @dlh

#14 @ianbelanger
8 weeks ago

  • Owner changed from nielslange to ianbelanger

#15 @ianbelanger
8 weeks ago

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

In 46813:

Bundled Themes: Adds Customizer option to show or hide author bio Twenty Twenty.

This adds an option to the Customizer that allows you to turn the author bio on or off, sitewide.

Props williampatton, nielslange acosmin, dlh.
Fixes #48550.

#16 @SergeyBiryukov
7 weeks ago

In 46848:

Twenty Twenty: Adds Customizer option to show or hide author bio.

This adds an option to the Customizer that allows you to turn the author bio on or off, sitewide.

Props williampatton, nielslange, acosmin, dlh, ianbelanger.
Merges [46813] to the 5.3 branch.
Fixes #48550.

#17 @SergeyBiryukov
7 weeks ago

@ianbelanger After a commit to trunk, please don't forget to reopen the ticket for minor release consideration, and add the `fixed-major` keyword.

Otherwise the ticket stays on the 5.3.1 milestone, but the changes are not actually included in 5.3.1. Thanks! :)

#18 follow-up: @ianbelanger
7 weeks ago

Thanks for the reminder @SergeyBiryukov. Do I need to do it on this ticket? Since it's already been backported.

#19 in reply to: ↑ 18 @SergeyBiryukov
7 weeks ago

Replying to ianbelanger:

Do I need to do it on this ticket? Since it's already been backported.

Just a note for the future :)

Note: See TracTickets for help on using tickets.