Filter Glyph for Comment Required Fields

[cais]

Currently the comment-template.php file uses an asterisk (*) as the default glyph for required fields used in the comment_form() function. This glyph is not easily manipulated without having to essentially over-write the entire comment_form() function.

I suggest the glyph be filtered. Therefore if one wants to change it, for example, to a hash (#) symbol then they can simply filter the output; or, if for any other reason one might want to enhance the glyph visibility or utility the filter would then allow for this while minimizing the impact on the default comment form.

[SergeyBiryukov]

This glyph is not easily manipulated without having to essentially over-write the entire comment_form() function.

This seems to work for me:

function change_required_fields_glyph_23870( $defaults ) {
	$defaults['fields']['author']     = str_replace( '*', '#', $defaults['fields']['author'] );
	$defaults['fields']['email']      = str_replace( '*', '#', $defaults['fields']['email'] );
	$defaults['comment_notes_before'] = str_replace( '*', '#', $defaults['comment_notes_before'] );
	return $defaults;
}
add_filter( 'comment_form_defaults', 'change_required_fields_glyph_23870' );

[cais]

Yes, that seems to work just fine. Although I still think the introduction of a direct filter for the glyph would be more inline with how the function was written and using a str_replace kluge doesn't look as pretty as would a more obviously named filter.

Either way, this solution or adding the filter would work (thus the reason for type: enhancement). Thanks for the snippet!

[cais]

Replying to SergeyBiryukov:

This seems to work for me:

To reach a point closer to my OP I would change your snippet to the following:

function required_fields_glyph_23870() {
	$glyph = apply_filters( 'comment_required_glyph_23870', '*' );
	return $glyph;
}
function change_required_fields_glyph_23870( $defaults ) {
	$defaults['fields']['author']     = str_replace( '*', required_fields_glyph_23870(), $defaults['fields']['author'] );
	$defaults['fields']['email']      = str_replace( '*', required_fields_glyph_23870(), $defaults['fields']['email'] );
	$defaults['comment_notes_before'] = str_replace( '*', required_fields_glyph_23870(), $defaults['comment_notes_before'] );
	return $defaults;
}
add_filter( 'comment_form_defaults', 'change_required_fields_glyph_23870' );

Usable but still not as clean as having the filter in core. Perhaps this can be cleaned up but that would be continuing even further off the idea I was suggesting originally of adding one line (the filtered variable) and modifying three others.

[SergeyBiryukov]

I'd suggest 'comment_form_required_field_mark' as the filter name, seems more obvious: 23870.patch​.

This could also probably be an additional comment_form() argument rather than a filter: 23870.2.patch​.

[cais]

I prefer '*_glyph' versus '*_mark' as I relate glyph to symbol (read: asterisk, hash, etc.) but either term is fine as they would both be used to accomplish the same goal of implementing this enhancement. I can also envision this to be used for much more than simply changing the character displayed so the naming convention may not be vitally important as is.

As to an argument rather than a filter ... I would probably think to look for a filter first (as I actually did) versus digging into the function to find the specific argument to address, but again either method is fine as both work. The argument method may be more inline with the rest of the function code even if I actually would prefer the filter myself.

[cais]

Going back to my original idea with the refreshed patch 23870.3.patch ...

[cais]

Refreshed patch file for comment-template.php

[rachelbaker]

@cais is this a request related to language translations? Would a translatable string be a better fit?

[johnbillion]

This ought to be a translatable string IMHO.

[cais]

@rachelbaker, @johnbillion - I don't think making the required "mark" part of the translation string would be the same as making it filter-able although an interesting idea to still take into consideration.

If the symbol is part of the translation string it could only be changed by over-writing the default comment strings as seen in the examples above which makes the idea of directly adding a filterable variable as moot and puts the idea of changing the symbol back on the theme or plugin developer that chooses to do so (while still allowing the translator to essentially overwrite it all the same?!)

The premise I was putting forward was to add more of a design element to the symbol versus just making it different (such as via a translation string). I do not have any specific use case where it has been changed and even though I have implemented a filterable method into one of my themes to allow the symbol to be changed I am not aware of any case where it actually has been changed, even by myself. The theme's implementation still uses the default asterisk.

I imagine the two ideas could still be put together and sorted out with a bit more refactoring but I may also have been a bit over-zealous with making "all the things" filter-able in the theme that gave me the impetus to suggest this enhancement.

[cais]

I've refreshed and adjusted the patch to include i18n relevant changes as well.

I also added a couple more conditional checks to insure required related items are only used if actually required. For example, if (for whatever reason) the get_option( 'require_name_email' ) is false then there is no reason to have any value for $required_text.

[cais]

refreshed patch including i18n changes

[SergeyBiryukov]

• Are we sure it should be a filter rather than a comment_form() argument?
• The filter name should start with comment_form rather than comment_fields, for consistency with other comment_form() filters.
• The new filter needs to be documented as per the ​WordPress PHP documentation standards.
• I agree the default string can be translatable, I don't see a reason to alter other strings here though.

Replying to cais:

I also added a couple more conditional checks to insure required related items are only used if actually required. For example, if (for whatever reason) the get_option( 'require_name_email' ) is false then there is no reason to have any value for $required_text.

This makes the check below redundant: ( $req ? $required_text : '' ), but I agree it's better to consolidate these checks in one place. $required_text should also be an empty string if $req is set, but $req_mark is empty.

Any objections to 23870.5.patch​?

[cais]

Adding the required mark as an argument does work, and the patch in general is obviously more to current standards (mea culpa, mea culpa). It seems the ticket has become more tightening up the comment form than just adding a way to easily change the required mark used ... I'm good with that, too!

[SergeyBiryukov]

Attached a version with the argument, just in case, so we could choose from both options :)

I'm ambivalent at the moment. Most themes only have one comment form, so the argument seems cleaner.

On the other hand, if a theme has more than one form, and/or utilizes other comment_form_* filters, the filter would probably make more sense.

Moving to the milestone to make a decision. I18N is not the main part of the ticket though, so moving back to the Comments component.

[SergeyBiryukov]

Related: #16206

[afercia]

Related: #37331

[jbpaul17]

Moving this out of the 4.7 milestone given today's Beta 1 window and no apparent movement on this ticket in the last two weeks.

[desrosj]

The most recent patch needs to be refreshed. This likely also needs some testing and someone to dig in document how any related changes in the last 6+ years affect any proposed ways to fix this.

[sabernhardt]

The proposed patch for helper functions on #54394 includes the translatable string mentioned above.

That can have a filter as well, though adding an argument option in the comments template could become trickier.

[sabernhardt]

r53888 added the translatable string.

#56389 proposes filters for the full markup of the indicator and message, but not the glyph on its own. If you have an opinion about that, please add your feedback there.

[sabernhardt]

With r54137, the wp_required_field_indicator filter is now available in trunk, so I recommend closing this ticket.

Core props could be added manually to the changesets, either the full group for both changes or divided by their contribution.
Filter (✅ added to r54137): cais, SergeyBiryukov, desrosj
Translatable string (✅ added to r53888): rachelbaker, johnbillion
Gardening: chriscct7, morganestes, afercia, jorbin, jeffpaul

[hellofromTonya]

I agree with @sabernhardt to close this ticket. I'm marking it as a duplicate of #56389.

Why close?

With #56389's changeset [54137], the glyph is now filterable through the 'wp_required_field_indicator' filter. As @sabernhardt notes, the new filter is for the entire required HTML, i.e. <span class="required">*</span>.

In my opinion, adding additional filter for only the glyth is not needed. The glyth itself is programmatically available to be changed, albeit a bit more code is needed (​some examples).

Why duplicate? Its scope was included in #56389.

Thank you everyone for your contributions to this ticket.