WordPress.org

Make WordPress Core

Opened 2 months ago

Closed 4 weeks ago

Last modified 4 weeks ago

#54229 closed enhancement (fixed)

Make dashboard widget control submit text filterable or change to "Save changes"

Reported by: zodiac1978 Owned by: hellofromTonya
Milestone: 5.9 Priority: normal
Severity: normal Version:
Component: Administration Keywords: has-patch commit
Focuses: ui-copy Cc:

Description

If a dashboard widget is using a control callback for configuration
_wp_dashboard_control_callback is used.

In this private function the submit button has a hardcoded "Submit" label.

As this is a configuration page, I always wondered why this does not use the wording "Save changes".

There is no (easy) way to change this label I am aware of.

Would it be useful to change this submit button text to "Save changes"? Or make this filterable?

In this context we could use the meta_box id to change the text just for one dashboard widget, so that we can remain backwards compatible and do not confuse people with changing the text for all dashboard widgets.

Attachments (2)

#54229.patch (524 bytes) - added by hilayt24 2 months ago.
Submit removed , save changes text added
54229.1.diff (636 bytes) - added by audrasjb 5 weeks ago.
Administration: Replace dashboard widget control "Submit" button text with "Save change" for consistency.

Download all attachments as: .zip

Change History (22)

#1 @SergeyBiryukov
2 months ago

  • Milestone changed from Awaiting Review to 5.9

#2 follow-up: @knutsp
2 months ago

All translatable texts are already filterable, I presume.

#3 in reply to: ↑ 2 ; follow-ups: @zodiac1978
2 months ago

Replying to knutsp:

All translatable texts are already filterable, I presume.

You are right, but even if we can filter via gettext this is not a good solution IMHO.

There is more than one occurrence of the string "Submit" in core:

wp-includes/js/dist/block-editor.js:34172
wp-includes/js/dist/block-editor.js:35432
wp-admin/includes/dashboard.php:245
wp-admin/includes/dashboard.php:1374
wp-admin/setup-config.php:252

See: https://translate.wordpress.org/projects/wp/dev/de/default/?filters%5Bstatus%5D=either&filters%5Boriginal_id%5D=510&filters%5Btranslation_id%5D=84288

Filtering the string via gettext would change all the places, or not? Or how could I limit the change to this one occurrence?

And there is no context used on those strings, which could be used.

#4 in reply to: ↑ 3 ; follow-up: @knutsp
2 months ago

Replying to zodiac1978:

Filtering the string via gettext would change all the places, or not? Or how could I limit the change to this one occurrence?

As with any hooked functios you can detect where you are in the function, or add the filter on another hook just before needed. I have not tested this exact case yet, but I do a lot of such conditional replacements since same English word may have to be translated to different words in output language.

Adding a context to this translatable string is the best way, IMO.

#5 in reply to: ↑ 4 @zodiac1978
2 months ago

Replying to knutsp:

Replying to zodiac1978:

As with any hooked functios you can detect where you are in the function, or add the filter on another hook just before needed. I have not tested this exact case yet, but I do a lot of such conditional replacements since same English word may have to be translated to different words in output language.

Looks like a complex solution to a simple problem.

Adding a context to this translatable string is the best way, IMO.

That would make it much easier to filter the translation but why not change the text completely?

We are talking about a control, so why do we just "Submit" a configuration change? The used submit_button() function has a default of "Save Changes". We could just remove the custom text "Submit" and go with "Save changes".

I still find it useful to customize this text easier. And core uses this in other places too (in excerpt_more for example).

#6 @hilayt24
2 months ago

@zodiac1978 , agreed I think it makes much sense with "Save changes".

@hilayt24
2 months ago

Submit removed , save changes text added

#7 @hilayt24
8 weeks ago

  • Keywords has-patch added; needs-patch removed

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


7 weeks ago

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


5 weeks ago

#10 follow-up: @audrasjb
5 weeks ago

  • Keywords needs-refresh added

The patch doesn't address all the occurrences, so let's refresh it

@audrasjb
5 weeks ago

Administration: Replace dashboard widget control "Submit" button text with "Save change" for consistency.

#11 in reply to: ↑ 3 @audrasjb
5 weeks ago

  • Keywords commit added; needs-refresh removed

54229.1.diff refreshes the previous patch.

Replying to zodiac1978:

There is more than one occurrence of the string "Submit" in core:

wp-includes/js/dist/block-editor.js:34172
wp-includes/js/dist/block-editor.js:35432
wp-admin/includes/dashboard.php:245
wp-admin/includes/dashboard.php:1374
wp-admin/setup-config.php:252

I didn't add a patch for other occurrences:

  • Block editor buttons need to be updated upstream on GitHub
  • The second dashboard.php occurrence is relevant in this case
  • The setup-config.php occurrence is relevant in this case

Marking this ticket ready for commit.

#12 follow-up: @hellofromTonya
5 weeks ago

  • Keywords reporter-feedback needs-patch added; has-patch commit removed

Removing commit keyword as 54229.1.diff does not address the 3 instances in Core.

Also needs consensus to agree on changing the submit_button text from Submit to
Save Changes vs making the submit_button's text filterable.

One way to make it filterable is to add a filter to get_submit_button() to filter the $text. However, in doing so, all submit buttons run through the filter, not just the ones mentioned in this ticket. I wonder: What value does the filter provide? Is it valuable for many folks?

@zodiac1978 @knutsp What do you think? Is there enough value in changing the button's text? If yes, is Save Changes the correct text for each of these instances?

#13 @knutsp
5 weeks ago

I'm all for better semantics and wording, where possible. "Save Changes" when that is what happens, especially settings or options that can be changed again later. "Submit" in cases where the input is just collected for processing or the result is more complicated or unclear - like in a setup process.

I'm all for string context, but keep same string/context across similar functions. "Dashboard Widget" should be (part of) the context of what this ticket focuses.

I'm not comfortable with adding special text filters here and there, as this ticket originally suggested, when we already have _x() - just make it work. I'm not sure about removing "Submit" from all over the codebase.

#14 in reply to: ↑ 12 @audrasjb
4 weeks ago

Replying to hellofromTonya:

Removing commit keyword as 54229.1.diff does not address the 3 instances in Core.

@hellofromTonya as mentioned above, I don't think we should change these 3 instances, because the current string is the most relevant in this specific use case.

#15 in reply to: ↑ 10 @zodiac1978
4 weeks ago

Removing commit keyword as 54229.1.diff​ does not address the 3 instances in Core.

I think there are some typos involved here:

The second dashboard.php occurrence is relevant in this case
The setup-config.php occurrence is relevant in this case

I think there is a "not" missing.

Block editor buttons need to be updated upstream on GitHub

The occurrences in Gutenberg shouldn't be changed IMHO.

wp-includes/js/dist/block-editor.js:34172
wp-includes/js/dist/block-editor.js:35432

These two are from the URL input modal for links in paragraphs or in buttons, therefore changing it to "Save changes" does not make sense as there are submitting a search or input and do not save a setting.

wp-admin/setup-config.php:252

This is from the installation process and submits the settings (database credentials, table prefix) and although these are saved, I think "Save changes" is not useful in this case.

wp-admin/includes/dashboard.php:245
wp-admin/includes/dashboard.php:1374

The first one is the generic dashboard widget control callback and as a control item I think "Save changes" makes more sense here. I want to change the setting for a dashboard widget and therefore change the settings for it. Just using "Submit" is not specific enough IMHO.

The second one is submitting the location for the event widget. As this is just one field, "submitting" the location feels fine for me. "Save changes" doesn't really fit here because of the plural ("changes").

This leaves us with just the already fixed occurrence for the widget control callback.

Adding a context gives us the opportunity to change the string in other languages for this occurrence without changing it elsewhere. But this still leaves the English button with "Submit" where "Save changes" makes much more sense as this is what is happening.

@hellofromTonya I don't think adding a filter is providing enough value here. I would recommend to just go with the latest patch as @audrasjb says.

Last edited 4 weeks ago by zodiac1978 (previous) (diff)

#16 @audrasjb
4 weeks ago

  • Keywords has-patch added; reporter-feedback needs-patch removed

Thanks for the detailed explanation @zodiac1978 💫

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


4 weeks ago

#18 @hellofromTonya
4 weeks ago

  • Keywords commit added
  • Owner set to hellofromTonya
  • Status changed from new to reviewing

#19 @hellofromTonya
4 weeks ago

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

In 52014:

Administration: Make dashboard widget control submit button text more clear.

Changes the submit button text from "Submit" to "Save Changes".

Why? The text is more semantic and clear of what happens when activating that button.

Follow-up to [9103].

Props zodiac1978, knutsp, hilayt24, audrasjb.
Fixes #54229.

#20 @hellofromTonya
4 weeks ago

Thank you @audrasjb @zodiac1978 @knutsp for your follow-up in helping to get consensus on the implementation. Thank you to everyone who contributed to this ticket! The fix has been committed and will ship with 5.9.

Note: See TracTickets for help on using tickets.