Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor settings Save button loading state #8730

Open
2 tasks
jimmymadon opened this issue May 19, 2024 · 1 comment
Open
2 tasks

Refactor settings Save button loading state #8730

jimmymadon opened this issue May 19, 2024 · 1 comment
Labels
P2 Low priority Type: Enhancement Improvement of an existing feature

Comments

@jimmymadon
Copy link
Collaborator

Feature Description

As per this internal Slack conversation, the following code snippet can be refactored. The resolution map should be removed from the generic Footer component and each module should implement its own selector to determine which selector should be resolved in order to determine whether it's settings have loaded or not.

// Check if the resolution for the specified selector has finished.
// This allows us to determine if the data needed by the module is still being loaded.
// The primary reason for this loading check is to disable the submit button
// while the necessary data for the settings is still being loaded, preventing
// premature interactions by the user.
const isLoading = useSelect( ( select ) => {
const resolutionMapping = {
analytics: 'getAccountSummaries',
tagmanager: 'getAccounts',
'search-console': 'getMatchedProperties',
};
const resolutionSelector = resolutionMapping[ slug ];
if ( ! module || ! resolutionSelector ) {
return false;
}
const storeName = module.storeName;
return ! select( storeName ).hasFinishedResolution(
resolutionSelector
);
} );


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • The code that determines the loading state within each module's settings footer (assets/js/components/settings/SettingsActiveModule/Footer.js) should be refactored so that:
    • Each module itself should implement its own selector which determines whether the module's settings would have been loaded or not. This information should then be used directly within this <Footer> component.

Implementation Brief

  • In assets/js/googlesitekit/modules/datastore/settings.js, add a new selector, isSettingsLoading. Similar to isDoingSubmitChanges, it should fetch the module datastore from the slug provided and call the selector of the same name from the individual module's datastore. Similarly, if the correct module-slugh or selector doesn't exist, it should just return false.
  • In assets/js/modules/analytics-4/datastore/settings.js, assets/js/modules/tagmanager/datastore/settings.js and assets/js/modules/search-console/datastore/settings.js, add a new selector that checks if the appropriate selector based on the resolutionMapping in assets/js/components/settings/SettingsActiveModule/Footer.js has finished resolution.

Test Coverage

  • Add tests for the new selectors similar to the tests for isDoingSubmitChanges().

QA Brief

Changelog entry

@jimmymadon jimmymadon assigned jimmymadon and unassigned jimmymadon May 19, 2024
@jimmymadon jimmymadon added P2 Low priority Type: Enhancement Improvement of an existing feature labels May 20, 2024
@tofumatt tofumatt self-assigned this May 23, 2024
@tofumatt
Copy link
Collaborator

Works for me 👍🏻

I see there's an IB here already that looks good, so moving this one right to the backlog 😄

IB ✅

@tofumatt tofumatt removed their assignment May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Low priority Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

2 participants