Make WordPress Core

#56365 closed enhancement (fixed)

Placeholders instead of values in setup-config.php

Reported by: oliverstapelfeldt's profile oliverstapelfeldt Owned by: oliverstapelfeldt's profile oliverstapelfeldt
Milestone: 6.1 Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: good-first-bug has-patch
Focuses: ui, accessibility Cc:

Description

Hello,

is there a reason why in setup-config.php for example in line 216 "value" is used instead of "placeholder"?
Because now when you click in the field you have to delete the value instead of just writing.

Best regards, Olli

Attachments (3)

56365.diff (780 bytes) - added by krupalpanchal 22 months ago.
Added patch
#56365.2.diff (2.8 KB) - added by oliverstapelfeldt 22 months ago.
#56365.3.diff (1.9 KB) - added by oliverstapelfeldt 22 months ago.

Download all attachments as: .zip

Change History (20)

#1 @audrasjb
22 months ago

  • Focuses ui accessibility added
  • Keywords needs-patch added; dev-feedback removed

Hello @oliverstapelfeldt, welcome back to WordPress Core Trac and thank you for opening this ticket,

This is a pretty old implementation, and I think it would indeed be good to challenge it again today :)

Would be nice to get a patch that replaces values with placeholder, just to make sure it doesn't introduce any backward compatibility issue :)

#2 @oliverstapelfeldt
22 months ago

I'm excited to try my first patch 😃

@krupalpanchal
22 months ago

Added patch

#3 follow-up: @audrasjb
22 months ago

@krupalpanchal when the person who created the ticket say "I'm excited to create my first patch", it is not super cool to send a patch of your own few hours later…

Fortunately, the above patch is missing a lot of fields available in the setup form. So @oliverstapelfeldt please feel free to create a patch of your own. You can do it using SVN or GIT, and by generating a diff file or via a GitHub pull request if you prefer 🙂

#4 @audrasjb
22 months ago

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

#5 @audrasjb
22 months ago

  • Keywords good-first-bug added

#6 in reply to: ↑ 3 @krupalpanchal
22 months ago

Replying to audrasjb:

@krupalpanchal when the person who created the ticket say "I'm excited to create my first patch", it is not super cool to send a patch of your own few hours later…

Sure @audrasjb 👍

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


22 months ago

#8 @audrasjb
22 months ago

  • Keywords 2nd-opinion added; needs-patch removed
  • Milestone changed from Awaiting Review to 6.1

Thanks for the patch!
By the way, I'm not sure we should use a placeholder for the table prefix as we need a default value.

#9 @audrasjb
22 months ago

  • Keywords has-patch added

#10 @sabernhardt
22 months ago

I think localhost is good as a default host value, too, not a placeholder.

Changing the table prefix has been recommended for security, but a default value of wp_ gives a suggestion for anyone who wants something else. Besides, the input field's prefix-desc description text includes "change this" as an option.

Also, these fields do not have required attributes.

#11 @oliverstapelfeldt
22 months ago

I'm not sure about required. Because then, if you leave Table Prefix empty, the following error message can be deleted.

This ticket was mentioned in Slack in #core-auto-updates by costdev. View the logs.


22 months ago

#13 @costdev
22 months ago

@sabernhardt What do you think about @oliverstapelfeldt's comment about required?

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


22 months ago

#15 @sabernhardt
22 months ago

  • Keywords 2nd-opinion removed

Perhaps we should stay focused on the placeholder attributes on this ticket. The fields have labels and instructions, which could be enough to avoid errors (yet also can be improved).

Required attributes might prevent the simple mistake of leaving one of the fields empty, but any incorrect value in the first four can result in a database connection error on the next screen.

And if the table prefix field becomes required, I would not want to remove the error message, in case someone tries to work around the required attribute.

This ticket was mentioned in Slack in #accessibility by sabernhardt. View the logs.


21 months ago

#17 @davidbaumwald
21 months ago

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

In 54231:

Upgrade/Install: Use "placeholder" for example values in setup-config.php.

During install, the user is prompted for database connection settings. Inputs for the database name, username, and password will most likely be changed from the examples given, but these example values are presented as the input's value property. This required the user to clear the current value before entering their own.

This change moves the example values for these fields to the placeholder property.

Props oliverstapelfeldt, audrasjb, krupalpanchal, sabernhardt.
Fixes #56365.

Note: See TracTickets for help on using tickets.