Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#54696 closed defect (bug) (fixed)

Names for new wp-login.php filter

Reported by: kebbet's profile kebbet Owned by: audrasjb's profile audrasjb
Milestone: 5.9 Priority: normal
Severity: normal Version: 5.9
Component: Login and Registration Keywords: has-patch commit has-dev-note
Focuses: administration, coding-standards Cc:

Description

I think the newly intruduced filters in 5.9 should be renamed for more consisteny with other filters in wp-login.php.

display_login_language_dropdown => login_language_display_dropdown
wp_login_language_switcher_args => login_language_switcher_args

Example of existing filters

  • login_enqueue_scripts
  • login_title
  • login_site_html_link
  • login_headerurl
  • login_headertext

Change History (26)

This ticket was mentioned in PR #2085 on WordPress/wordpress-develop by kebbet.


2 years ago
#1

  • Keywords has-patch added; needs-patch removed

#2 @SergeyBiryukov
2 years ago

  • Milestone changed from Awaiting Review to 5.9

#3 follow-up: @kebbet
2 years ago

Or possibly login_display_language_dropdown.

#4 in reply to: ↑ 3 @mukesh27
2 years ago

This make sense for me.

Replying to kebbet:

Or possibly login_display_language_dropdown.

#5 @audrasjb
2 years ago

  • Keywords needs-dev-note added

#6 @audrasjb
2 years ago

  • Owner set to audrasjb
  • Status changed from new to reviewing

#7 @audrasjb
2 years ago

  • Keywords needs-refresh added

This change looks relevant to me, even if we would still have some inconsistent filters like wp_login_errors.

Also, login_display_language_dropdown seems better than login_language_display_dropdown.

#8 @audrasjb
2 years ago

  • Keywords needs-refresh removed

PR updated to use login_display_language_dropdown.

#9 @audrasjb
2 years ago

My only concern with the proposed name for the filter is that the order of the words in "login_display_language_dropdown" doesn't look "natural".
@SergeyBiryukov do we have a best practice for this?

There is nothing about word order in the PHP coding standards handbook page: https://developer.wordpress.org/coding-standards/wordpress-coding-standards/php/

#10 @kebbet
2 years ago

Maybe remove display from name? Or add something with visibility at the end and remove display?

#11 @Clorith
2 years ago

login_display_language_dropdown seems correct to me, login_ prefixing the pålacement of the filter, and then the "descriptive" naming of "display language dropdown" yes/no (true/false).

#12 @hellofromTonya
2 years ago

  • Keywords commit added

I agree. login_display_language_dropdown looks good to me too. The login_ prefix is consistent with many of the other filters in the same file. Marking PR 2085 for commit.

#13 @audrasjb
2 years ago

Alright, thank you @hellofromTonya and @Clorith, I'll commit this right now 👍

#14 @audrasjb
2 years ago

I'll update the related dev note once it's committed.

#15 @audrasjb
2 years ago

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

In 52432:

Login and registration: Rename two filters related to language dropdown for better consistency.

This change renames two filters introduced in WP 5.9 development cycle. It replaces display_login_language_dropdown with login_display_language_dropdown and wp_login_language_switcher_args with login_language_switcher_args, for better consistency with the other existing login_* filters.

Props kebbet, mukesh27, audrasjb, Clorith, hellofromTonya.
Fixes #54696.

#17 @audrasjb
2 years ago

  • Keywords has-dev-note added; needs-dev-note removed

#18 @SergeyBiryukov
2 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Sorry for reopening again, could we rename login_language_switcher_args to login_language_dropdown_args?

That would be more consistent with some other *_dropdown_args filters:

  • taxonomy_parent_dropdown_args
  • post_edit_category_parent_dropdown_args
  • widget_archives_dropdown_args
  • widget_categories_dropdown_args

It would also be more consistent with the new login_display_language_dropdown filter, which uses the dropdown term instead of switcher.

#21 @audrasjb
2 years ago

  • Keywords commit removed

Good point @SergeyBiryukov!

Thanks for the PR @faisal03, it looks good to me.

#22 @audrasjb
2 years ago

  • Keywords commit added

Checks are passing on the PR, I'll commit this right now.

#23 @audrasjb
2 years ago

  • Keywords needs-dev-note added; has-dev-note removed

Re-adding needs-dev-note keyword, to not forget to update the existing devnote.

#24 @audrasjb
2 years ago

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

In 52435:

Login and registration: Rename login_language_switcher_args to login_language_dropdown_args.

This change adds better consistency with the other *_dropdown_args filters, like taxonomy_parent_dropdown_args or widget_archives_dropdown_args. It is also more consistent with the new login_display_language_dropdown filter.

Props SergeyBiryukov, faisal03.
Fixes #54696.

#25 @audrasjb
2 years ago

  • Keywords has-dev-note added; needs-dev-note removed
Note: See TracTickets for help on using tickets.