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

Save all fields to localStorage #24

Merged
merged 7 commits into from
Aug 20, 2023
Merged

Save all fields to localStorage #24

merged 7 commits into from
Aug 20, 2023

Conversation

cristianmacedo
Copy link
Contributor

@cristianmacedo cristianmacedo commented Aug 20, 2023

Closes #4

As highlighted by @chriskthomas in issue #4, right now, all input values are lost upon page refresh or revisiting the page later. To address this, I've implemented a localStorage saving mechanism with the following steps:

  1. LocalStorage Data Storage
    Upon pressing the "Submit" button, all form values are now saved to a form localStorage item. This is achieved using the FormData interface, which conveniently captures the key/value pairs of form fields and their corresponding values.

    To adhere to the Input HTML Specification saying that a file input "Throws an "InvalidStateError" DOMException if it is set to any value other than the empty string" I've excluded file input values from being saved to localStorage. A localStorageIgnore list has been introduced to handle this exclusion. For the best approach, we could explore dynamically excluding any input field that isn't an instance of a File object before saving.

  2. Restoring Saved Values
    When the page loads, the values stored in the form localStorage item are automatically populated into their respective input fields based on the name property. Additionally, for input fields with the prefix links that are not already present in the document, and have an index higher than the current index of existing links, a function is triggered to create the missing input fields. This ensures that additional links saved to localStorage are correctly reconstructed upon page load.

  3. Clear Functionality
    I've implemented a "Clear" button located next to the "Submit" button. This button, when clicked, utilizes the form.reset() method to clear all form values. Additionally, any data stored in the form localStorage item is removed, providing a clean slate for the user.

    The choice to use form.reset() over reloading the entire page was made to maintain a smooth user experience without unnecessary page refreshes.

I welcome feedback and suggestions on the dynamic approach for handling file inputs and any other parts of the implementation. By the way, I've gone ahead and made a few adjustments to variable names and aspects of code-splitting as well as some JSDoc comments to some functions. I hope that's alright – just aiming to enhance the overall structure.

Copy link

@accesslint accesslint bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are accessibility issues in these changes.

src/api.php Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
@mwt
Copy link
Collaborator

mwt commented Aug 20, 2023

Let's give it a try!

@mwt mwt merged commit 6581315 into chriskthomas:main Aug 20, 2023
1 check passed
@mwt
Copy link
Collaborator

mwt commented Aug 20, 2023

It's live at the test domain: https://linkfree.fly.dev

It seems to be working. I haven't read the code yet. It seems good.

The preview checkbox doesn't get saved. I think this is good. Is this also explicitly excluded?

@cristianmacedo
Copy link
Contributor Author

HI @mwt, thanks for merging the PR, it seems to be working great on the production environment.

Regarding the preview button, from my understanding that's actually a bug. The localStorage script tries to set the input type="checkbox" value to "on", but the property that should be updated so that the checkbox turns into, well... checked, is the checked property, which should be changed to "true". Not sure why the value property doesn't work. From my understanding this is not a problem right now but let me know if we want to work around that. Thanks!

@mwt
Copy link
Collaborator

mwt commented Aug 21, 2023

Hi. That makes sense. I think the ideal is for all the fields that are intended to be saved to have a .saved class or a data-saved=true property so that any input without this property is ignored

Merging the PR actually puts it in a dev environment. I have to sync to the production branch for it to affect the production version.

It's a bit confusing, but:

Dev: https://linkfree.fly.dev

Production: https://linkfree.ckt.im

@cristianmacedo
Copy link
Contributor Author

Ah yes I didn't notice it was the development environment.
That sounds good to me, I will work on a new change to only save to localStorage fields with the specified properties (.saved class or data-saved=true) as soon as possible, and I will be back with a new PR.
Thanks!

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.

Save all fields to localStorage
2 participants