LYS - Disable the save changes button until changes are made #47316

moon0326 · 2024-05-09T12:23:12Z

Submission Review Guidelines:

I have followed the WooCommerce Contributing Guidelines and the WordPress Coding Standards.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have reviewed my code for security best practices.
Following the above guidelines will result in quick merges and clear and detailed feedback when appropriate.

Changes proposed in this Pull Request:

Closes #47304

This PR disables the Save changes button until changes are made.

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

Create a new JN site with this branch.
Go to WooCommerce -> Settings -> Site Visibility
Confirm the Save changes button is in disabled state.
Make changes.
Confirm the button changes to enabled.
Make changes back to the original.
Confirm the button is back to disabled state.

Changelog entry

Automatically create a changelog entry from the details below.

Significance

Patch
Minor
Major

Type

Fix - Fixes an existing bug
Add - Adds functionality
Update - Update existing functionality
Dev - Development related task
Tweak - A minor adjustment to the codebase
Performance - Address performance issues
Enhancement - Improvement to existing functionality

Message

LYS: disables the "Save changes" button until changes are made.

Comment

github-actions · 2024-05-09T12:23:31Z

Hi @adrianduffell, @rjchow, @woocommerce/ghidorah

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

github-actions · 2024-05-09T12:29:30Z

Test using WordPress Playground

The changes in this pull request can be previewed and tested using a WordPress Playground instance.
WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Test this pull request with WordPress Playground.

Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit.

psealock · 2024-05-09T19:20:21Z

plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/api/plugins.php

@@ -111,6 +110,7 @@ public function test_activate_plugin() {
 		$response       = $this->server->dispatch( $request );
 		$data           = $response->get_data();
 		$active_plugins = Plugins::get_active_plugins();
+		var_dump($data);


@psealock Thank you for catching it 😄 Removed it in Remove test code 🙏

rjchow · 2024-05-10T02:58:12Z

plugins/woocommerce-admin/client/launch-your-store/settings/slotfill.js

+		const initValues = {
+			comingSoon: setting.woocommerce_coming_soon,
+			storePagesOnly:
+				setting.woocommerce_store_pages_only === false ? 'no' : 'yes',


I think this checking for === false is causing the save button to be disabled only when all the values are 'yes', so it's not actually disabling the save button for any other combination of values

@rjchow Thanks! I'll re-test it.

@rjchow Fixed in Fix incorrect comparison

rjchow · 2024-05-10T03:01:18Z

plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/api/plugins.php

 		$request = new WP_REST_Request( 'POST', $this->endpoint . '/activate' );
+		if ( ! is_plugin_active( 'woocommerce/woocommerce.php' ) ) {


is this for fixing this test failure? #47337

@rjchow Yeah, good find! I missed that. I'll rebase instead 👍

…mmerce

rjchow

lgtm, thanks!

chihsuan

Tested well. Left a comment to avoid adding more lodash functions.

Also, I agree with @psealock that the Save button behavior should be consistent across all settings screens. I think we should have a solution that works for all settings screens, not just this one. But that can be a separate task.

chihsuan · 2024-05-13T03:37:49Z

plugins/woocommerce-admin/client/launch-your-store/settings/slotfill.js

 } from '@wordpress/element';
 import { registerPlugin } from '@wordpress/plugins';
 import { __ } from '@wordpress/i18n';
 import classNames from 'classnames';
 import { useCopyToClipboard } from '@wordpress/compose';
 import { recordEvent } from '@woocommerce/tracks';
 import { getSetting } from '@woocommerce/settings';
+import { isEqual } from 'lodash';


We can replace isEqual with fastDeepEqual so we don't have to add more lodash functions to our codebase (pdToLP-yS-p2).

@chihsuan TIL! Replaced it with simple === instead in 6e72292

adrianduffell · 2024-05-13T07:23:26Z

Also, I agree with @psealock that the Save button behavior should be consistent across all settings screens. I think we should have a solution that works for all settings screens, not just this one. But that can be a separate task.

I've now created a follow-up issue for this #47393.

moon0326 requested review from a team, adrianduffell and rjchow May 9, 2024 12:23

moon0326 closed this May 9, 2024

moon0326 reopened this May 9, 2024

github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label May 9, 2024

psealock reviewed May 9, 2024

View reviewed changes

rjchow requested changes May 10, 2024

View reviewed changes

rjchow reviewed May 10, 2024

View reviewed changes

moon0326 and others added 8 commits May 10, 2024 08:29

Disable the save changes button until changes are made

19f3fb5

Add changefile(s) from automation for the following project(s): wooco…

f2dd68e

…mmerce

Fix broken test

fae1b47

Test

7abd6e8

Remove test code

21425ff

Tmp fix - fix broken test

31910ae

Fix incorrect comparison

edbceef

Remove test code

b8990c3

moon0326 force-pushed the update/47304-disable-save-button-until-changes-are-made branch from a784cbc to b8990c3 Compare May 10, 2024 15:30

minor refactor -- re-use setting

8d7c522

rjchow approved these changes May 13, 2024

View reviewed changes

chihsuan reviewed May 13, 2024

View reviewed changes

adrianduffell mentioned this pull request May 13, 2024

Add disabled state to save button on all settings tabs #47393

Open

Remove use of loadash isequal

6e72292

moon0326 merged commit 5183b1b into trunk May 13, 2024
24 of 25 checks passed

moon0326 deleted the update/47304-disable-save-button-until-changes-are-made branch May 13, 2024 14:56

github-actions bot added this to the 9.0.0 milestone May 13, 2024

github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label May 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LYS - Disable the save changes button until changes are made #47316

LYS - Disable the save changes button until changes are made #47316

moon0326 commented May 9, 2024

github-actions bot commented May 9, 2024

github-actions bot commented May 9, 2024 •

edited

psealock May 9, 2024

moon0326 May 9, 2024

rjchow May 10, 2024

moon0326 May 10, 2024

moon0326 May 10, 2024 •

edited

rjchow May 10, 2024

moon0326 May 10, 2024

rjchow left a comment

chihsuan left a comment

chihsuan May 13, 2024

moon0326 May 13, 2024

adrianduffell commented May 13, 2024

		$request = new WP_REST_Request( 'POST', $this->endpoint . '/activate' );
		if ( ! is_plugin_active( 'woocommerce/woocommerce.php' ) ) {

LYS - Disable the save changes button until changes are made #47316

LYS - Disable the save changes button until changes are made #47316

Conversation

moon0326 commented May 9, 2024

Submission Review Guidelines:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Changelog entry

Significance

Type

Message

Comment

github-actions bot commented May 9, 2024

github-actions bot commented May 9, 2024 • edited

Test using WordPress Playground

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moon0326 May 10, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rjchow left a comment

Choose a reason for hiding this comment

chihsuan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrianduffell commented May 13, 2024

github-actions bot commented May 9, 2024 •

edited

moon0326 May 10, 2024 •

edited