[BBT-83] Reset/Save button functionality enhancements

[Joe-Rouse]

Description

Completes BBT-83 - This PR makes some improvements to the functionality of the current save & reset buttons. The ticket outlines issues with the reset button not resetting the components, but I can't recreate this issue. However, this ticket was made nearly 4 months ago so I understand it could be a little outdated. I've completed all other suggestions in the main ticket description & comments.

Change Log

• Added functionality for displaying an alert if the user tries to navigate away from/refresh the page with unsaved changes.
• Disables the 'Reset' button if there are no changes & the user config is no different from the base config. (using lodash isEqual for this comparison).
• Disables the 'Save' button if there are no changes.

Steps to test

• Load Themer and make a change without saving.
• Try and navigate away from the page or refresh the page. Check you get an alert.
• Dismiss the alert and click save. Check the 'Save' button is disabled & the 'Reset' button is active.
• Click the 'Reset' button. Check the 'Reset' button is now disabled too.
• Try navigate away from the page. Check you don't get an alert.

Screenshots/Videos

Alert

Screen.Recording.2023-10-11.at.15.11.51.mov

Button disabled status

Screen.Recording.2023-10-11.at.15.13.38.mov

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• Any dependent changes have been merged and published in downstream modules

[g-elwell]

From the ticket description, it looks like fields not updating when reset is pressed has been resolved in recent work, but we do also want to update the reset button to revert to the previously saved state (not the original theme.json setting) when clicked. Can you look into adding this?

Expectation:

• theme.json has background-color: red
• update background-color to green via UI
• save
• update background-color to blue via UI
• click "reset"
• background-color should revert to green, not red

We will likely move the "reset to theme.json" functionality to the settings dropdown alongside "export" but not expecting that to be done here.

[Joe-Rouse]

@g-elwell Sorry I've misunderstood the ticket requirements there.

I've now implemented this logic by adding a new state variable, lastSavedUserConfig, and using that when running the reset function. I'm also using this when determining whether to disable the reset/save buttons or not.

[g-elwell]

I think we can simplify this a lot by using getEntityRecord to retrieve the original user config.

eg:

const { globalStylesId, baseConfig, userConfig, savedUserConfig } =
	useSelect( ( select ) => {
		const {
			__experimentalGetCurrentGlobalStylesId,
			__experimentalGetCurrentThemeBaseGlobalStyles,
			getEditedEntityRecord,
			getEntityRecord,
		} = select( 'core' );

		const currentGlobalStylesId =
			__experimentalGetCurrentGlobalStylesId();

		return {
			globalStylesId: currentGlobalStylesId,
			baseConfig: __experimentalGetCurrentThemeBaseGlobalStyles(),
			userConfig: getEditedEntityRecord(
				'root',
				'globalStyles',
				currentGlobalStylesId
			),
			savedUserConfig: getEntityRecord(
				'root',
				'globalStyles',
				currentGlobalStylesId
			),
		};
	} );

We are currently using getEditedEntityRecord for userConfig, and editEntityRecord to update this value whenever the UI is interacted with. This keeps the theme settings in state but won't update the entity until we call saveEditedEntityRecord when the save button is pressed.

So, we should be able to compare the original/saved entity with our "live"/edited entity to determine if things are dirty.

If this works, it should remove the need for any additional state or manipulating objects to compare them.

The behaviour now is spot on though, both the reset/save and warning when leaving the page 👍

[Joe-Rouse]

@g-elwell Thanks, I've got a better understanding of where this data is being saved now. I've made those changes so there's no need for the extra state I was creating before.

[g-elwell]

Works great, thanks @Joe-Rouse !