WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 2 months ago

#16886 closed enhancement (fixed)

Adding readonly function to wp-includes/general-template.php

Reported by: kakidcm Owned by: SergeyBiryukov
Milestone: 4.9 Priority: normal
Severity: minor Version: 3.1
Component: General Keywords: 2nd-opinion has-patch needs-testing
Focuses: Cc:

Description

A quite simple improvement, really.

Just like the checked, selected and disabled function, readonly would be a shortcut to checked_selected_helper.

I was doing a theme option page and realized that disabling fields removes the data from the data submission.

I have a bunch of fields that gets used (or not) in the theme, depending on a on/off switch. At first, depending on the switch, I was disabling the fields, but all the data is removed on submit if the switch is at off. Meaning that the user would have to reenter every fields if he wants to activate the functionality.

Putting the fields on readonly gives a nice feedback to the user that those fields are not use, depending on the switch, AND the data is kept if the user changes his mind.

So, here's the patch.

/**
 * Outputs the html readonly attribute.
 *
 * Compares the first two arguments and if identical marks as readonly
 *
 * @since 3.1.#
 *
 * @param mixed $disabled One of the values to compare
 * @param mixed $current (true) The other value to compare if not just true
 * @param bool $echo Whether to echo or just return the string
 * @return string html attribute or empty string
 */
function readonly( $readonly, $current = true, $echo = true ) {
    return __checked_selected_helper( $readonly, $current, $echo, 'readonly' );
}

Attachments (1)

16886.patch (1.5 KB) - added by soulseekah 4 months ago.

Download all attachments as: .zip

Change History (15)

#1 @hakre
7 years ago

The function name is overly broad, what about renaming the function to output_html_readonly_attribute()?

Next to that what is the need of the aliasing here? You could do in your own plugin which would spare you the typing as well (or even better you can create a DSL and then even more easily write your code).

#2 @hakre
7 years ago

Related: #11517

#3 @c3mdigital
4 years ago

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

#4 @helen
4 years ago

  • Keywords needs-patch 2nd-opinion removed
  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Going to echo azaozz's sentiment on #20110:

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.

#5 @mihdan
4 months ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

#6 follow-up: @soulseekah
4 months ago

Can we try again? Seems like a useful thing. We have disabled which is almost the same but readonly will submit the fields. I think there's a legitimate use case here. It's not like we're asking for required() :)

#7 @SergeyBiryukov
4 months ago

  • Milestone set to Awaiting Review

#8 in reply to: ↑ 6 @SergeyBiryukov
4 months ago

  • Keywords needs-patch 2nd-opinion added

Replying to soulseekah:

Seems like a useful thing. We have disabled which is almost the same but readonly will submit the fields.

Agreed, readonly isn't that much different from disabled, makes sense to have both for consistency.

@soulseekah
4 months ago

#9 @SergeyBiryukov
4 months ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Awaiting Review to 4.9

#10 @SergeyBiryukov
4 months ago

Checked the Plugin Directory for potential conflicts, haven't found any.

#11 @jbpaul17
3 months ago

  • Keywords needs-testing added

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


3 months ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


3 months ago

#14 @SergeyBiryukov
2 months ago

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

In 41728:

Template: Introduce readonly() form helper to complement the disabled() helper added in [13658].

Props soulseekah.
Fixes #16886.

Note: See TracTickets for help on using tickets.