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

Feat/warning if unsaved settings #288

Merged
merged 6 commits into from
Dec 9, 2020

Conversation

kwajiehao
Copy link
Contributor

This is a replica of #285 since we merge-committed into staging instead of squashing and merging, which resulted in a messy history.

Jie Hao Kwa and others added 6 commits December 8, 2020 23:12
This commit saves the original settings configuration as a state variable
so that we can compare the current settings fields with the original
state to see if any changes were made. This allows us to warn the user
if they attempt to navigate away from the page without saving their
changes.

This commit also removes the `footerSha` attribute from state since
it is no longer required after PR #85 on the backend.
This commit introduces a util function i found on stack overflow,
https://stackoverflow.com/a/39813128, which deep compares two objects and
identifies which attributes have changed.
This commit deep compares the current settings values with the original
loaded settings values and displays a warning if the user attempts to
navigate away from the page when there are unsaved changes.
This commit modifies the behavior of the Settings layout save handler
so that only fields which have been changed are sent in the request.
Previously, we would send all state fields, regardless of whether the
fields have changed in value. The problem is that this results in
wasteful writes to the GitHub repo on the backend. It also leads to
slow performance since the writes to the GitHub repo are sequential.

Co-authored-by: Jie Hao Kwa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants