Feat: Add disable functionality to save settings button (#1531)

[hbhalodia]

All Submissions:

• Have you followed the Newspack Contributing guideline?
• Does your code follow the WordPress' coding standards and VIP Go coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

• By adding this PR, we can ensure that save settings button is disabled until user changes any settings in a particular section. Before this commit save settings button was always enabled. This would only enable the save settings button for the section whose inner settings are changed, that would ensure all the sections on the page are async.
• Also on this PR in file change index.js, we need to look upon the comment added on handleSectionUpdate function, because we have kept all sections save settings button async, hence user can change all settings at once and can try to update one by one which would override his settings because on update we are fetching the saved settings and adding it again on the response. We can remove this behavior by removing the setState again for saved settings instead use prev state only.

How to test the changes in this Pull Request:

1. Checkout to the current branch.
2. Head to Advertising --> Settings page.
3. On first load you can see the button is disabled, Change any section settings you can see that section button is enabled, On save again button gets disabled.
4. You can check with multiple section and each section works async.

Screencast for the current change

Untitled_.Jan.16.2023.5_28.PM.webm

Other information:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes, as applicable?
• Have you successfully ran tests with your changes locally?

Closes - #1531

[leogermani]

Thank you for this!

It works as described but I left one comment about the button not being disabled while saving.

Also, it doesn't seem to apply to all SettingsSection components. If you go to Engagement > Social, you'll see that this will not work with the sections there. Apparently Ads settings are created in a very different way. cc @miguelpeixe

[hbhalodia]

Hi @leogermani, You need me to update for engagement > social settings also or we can keep this just for ads, as you said ads settings are created differently? Let me know on this. Thanks.

I think we can keep this for ads right now as this PR is created for the issue which has only advertisement issue. We can create another issue for that settings and should solve for all in that?

[hbhalodia]

Hi @leogermani, I have added for engagement settings also. We need to apply this change where PluginSettings component is being used. Thanks.

[miguelpeixe]

Thank you for your contribution, @hbhalodia!

May I suggest a different approach? The SettingsSection component is aware of the value changes and saves so we don't need to rely on another component to set this:

newspack-plugin/assets/components/src/plugin-settings/SettingsSection.js

Lines 71 to 73 in 6904356

	onChange: value => {
	onChange( setting.key, value );
	},

newspack-plugin/assets/components/src/plugin-settings/SettingsSection.js

Lines 104 to 106 in 6904356

	onClick={ () => {
	onUpdate();
	} }

With the current approach, every component that uses this will have to reimplement the same (or very similar) logic for a consistent experience. Case in point, the Ad Refresh Control is missing it, so that's how my Ads settings render currently:

[hbhalodia]

Thanks! @miguelpeixe for the approach. I have worked on this approach and achieved the results for all settings. No need to reimplement the same logic again on each component. I have updated the PR with the new approach. You can review now.

Thanks!

[miguelpeixe]

Thank you for revising, @hbhalodia!

It's working well 👌

[matticbot]

🎉 This PR is included in version 1.106.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

[matticbot]

🎉 This PR is included in version 1.106.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀