WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#20110 closed enhancement (wontfix)

Add a valued() function to encourage consistency in input fields

Reported by: jeremyfelt Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Template Keywords: close
Focuses: Cc:

Description (last modified by helenyhou)

A valued() function would fit well with the helper functions selected() and checked().

Makes it easier to add a value if it exists, and to escape the value with esc_attr().

Makes it easier to provide a placeholder text.

Use would be:

<input type="text" <?php valued( $possible_num, true, '4 digit number' ); ?>>

Which would result in:

<input type="text" value="1234" placeholder="4 digit number">

Or

<input type="text" placeholder="4 digit number">

(Depending on the original value of $possible_num)

Attachments (2)

20110.diff (1.0 KB) - added by jeremyfelt 2 years ago.
20110.2.diff (1.0 KB) - added by jeremyfelt 2 years ago.
Updated function

Download all attachments as: .zip

Change History (20)

jeremyfelt2 years ago

comment:1 helenyhou2 years ago

  • Description modified (diff)

comment:2 scribu2 years ago

  • Keywords 2nd-opinion added

I don't really see the benefit. Unlike checked and selected, writing value="" has no adverse effects, so there's no if statement necessary.

comment:3 solarissmoke2 years ago

Also, the proposed function would fail if you supplied it a falsy value, e.g., '0'.

(by fail I mean it wouldn't output a value when it should output "value='0'")

Last edited 2 years ago by solarissmoke (previous) (diff)

jeremyfelt2 years ago

Updated function

comment:4 jeremyfelt2 years ago

Good catch. New diff attached with fixed function that handles 0 correctly for both $value and $placeholder.

I think the benefit comes when writing several input fields for meta boxes. Would make code cleaner to read and easier to keep consistent.

comment:5 nacin2 years ago

Core doesn't use placeholder values, so I'm not sure how useful this is yet.

comment:6 ocean902 years ago

Would make code cleaner to read

Really? With your function you need to check first if a placeholder or a value attribute is meant. And I'm not a fan of the name valued.

comment:7 scribu2 years ago

This doesn't seem more readable to me:

<input type="text" <?php valued( $possible_num, true, '4 digit number' ); ?>>

I have no idea what the second and third parameters do, unless I look up the definition for the valued() function.

The placeholder attribute has no place in the valued() function.

comment:8 follow-up: scribu2 years ago

In general, let's not make the same mistake we made with submit_button(), where we ended up with code like this:

submit_button( __( 'Apply' ), 'button-secondary action', false, false, array( 'id' => "doaction$two" ) );

You get +100 psychic points if you can guess what those two false values represent, without looking at the definition.

comment:9 in reply to: ↑ 8 jeremyfelt2 years ago

You get +100 psychic points if you can guess what those two false values represent, without looking at the definition.

That could be the case with many functions. I think the true/false for echo/return works here.

I do think placeholder is a useful addition for UI purposes, but see how it could be considered fluff in a function that could provide values for more than type="text".

With the default values of the function though, only this is required:

<input type="text" <?php valued( $possible_num ); ?>>

I don't have any reasons to leave out a blank value="", so it may be that the if statement is unnecessary. Of course once you take that out, it appears I may be overreaching a bit.

comment:10 follow-up: ocean902 years ago

What should I do if I want placeholder and value?

comment:11 in reply to: ↑ 10 ; follow-up: jeremyfelt2 years ago

Replying to ocean90:

What should I do if I want placeholder and value?

You would supply 3 arguments.

comment:12 in reply to: ↑ 11 ocean902 years ago

Replying to jeremyfelt:

You would supply 3 arguments.

Oops. Yes, I misread the function. :)

comment:13 nacin2 years ago

You get +100 psychic points if you can guess what those two false values represent, without looking at the definition.

I know that one controls <p> tags being wrapped or not, but I didn't know which. It turns out the other one is the name attribute.

That could be the case with many functions. I think the true/false for echo/return works here.

This works sometimes, but it gets old really quick.

In fairness, valued()'s third argument is the same as the third argument for checked() and selected() — whether to echo. Unfortunately, the first and second arguments do not have parity. In checked(), selected(), and disabled(), the values are compared to ascertain whether to issue the corresponding selector. They are designed to help with loops because the code gets messy here quick.

Not to pick at a function name itself, but neither attribute is called valued, while they are called checked, selected, and disabled. All three of those are also boolean attributes that control the input state, not something that requires a value.

If we are to consider a valued() function, we might as well just have an input() function that serves up the whole element. Generally, I think HTML is enough of an "API" all on its own.

comment:14 azaozz2 years ago

Replying to scribu:

In general, let's not make the same mistake we made with submit_button()...
...You get +100 psychic points if you can guess what those two false values represent...

Agreed. Don't see a point in replacing simple HTML attributes with PHP functions that generate them. This type of functions would always need lots of args to be able to cover the more common cases which makes the code a lot harder to read. On top of that they usually don't cover all possible cases making them redundant.

comment:15 azaozz2 years ago

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

comment:16 scribu2 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

comment:17 scribu2 years ago

That said, I have a generic html() function that takes care of using the appropriate esc_*() helpers. I'm still not sure if it was a good idea, though.

comment:18 jeremyfelt2 years ago

All good points, I'm convinced. Thanks for taking a look, all.

The only case where this now seems to make sense is the one esc_attr() can already handle, creating redundancy.

Note: See TracTickets for help on using tickets.