-
Notifications
You must be signed in to change notification settings - Fork 98
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
Remember input in LocalStorage #1382
Conversation
@jotoeri @Chartman123 Can you please help figuring out how to change QuestionDate State on mounted() :) |
Nice start! |
@Chartman123 @susnux @skjnldsv maybe you can help? :) @jotoeri all the best for the thesis! :) |
I will try to find some time after the weekend :) |
Nice start on this one!
Currently this is not possible, as I filed a PR for initializing those question types from the values property as well ( #1383 ), which should make this more easy. I did not have a in depth view on your changes, but one thing I was thinking about is: |
@susnux Hey good catch with the multiple forms issue, I didn't pay attention to this scenario. I'll update the PR and Fix the failing Checks. |
22659d0
to
9baa9ac
Compare
91571ed
to
7758c4a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @hamza221! Nice to see you back here 🙂
I don't know much about LocalStorage. However, I think that this is not working as it's intended. I can only see the stored values as long as I navigate within the same form (e.g. between submit - edit - submit). As soon as I leave the form and go to another one and then come back all fields are empty again. As soon as I then do a reload on the page, the values are loaded.
Also could you please do a rebase on current main as we have quite some newer versions in composer.lock 👍🏻
Thank you @Chartman123
|
c6d33a2
to
143ddce
Compare
On a public share link Furthermore the array still has lots of @jancborchardt Should we also add a "Reset Form" button to clear the LocalStorage and start fresh? |
The null, and empty hash are solved, @jancborchardt should we add a Reset Form button? |
@hamza221 Looks way better now :) However, when you switch the form within one session you end up with multiple records with duplicated contents: Looks like we need to reset the object between opening different forms so that the old contents don't get written to the content of another form. I think we need to move the init function away from "beforeMount". Also, I think that we can get rid of the "id" key inside the object as you're already using it as a key in the top object. And please rebase on current main 🙂 |
bb29fd3
to
9abbf5d
Compare
same job as #1416 failing :/ |
The failed jobs must be something general with the images... I also tried to use the same as in the mail repo, but that didn't help either. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good now to me. 👍🏻
Basically this is good now 👍 But there is one thing I am unsure about: Do we want to store values on public shares for anonymous users? |
Yes, that's a valid point. What about combining this here with my started PR regarding the warning when you leave an unsubmitted form? So that we only store the values to the local storage when the user is leaving an unsubmitted form and chooses to save them? |
@hamza221 Submit.vue now handles leaving the form with unsubmitted changes. You could try to update this PR so that data will only be stored in local storage when the user leaves the form without a submission and answers the dialog accordingly |
@Chartman123 I'm not sure I understand how this will fix @susnux 's concern. So if I understand correctly ->
Or do you mean we save it when he chooses to stay? If so I'm not sure how is that useful. What about saving the answers only if the user is connected and showing the warning otherwise? I'm sorry I'm just trying to figure out what's the most useful path we should take :)) Update: Google Forms only saves for logged in users and only shows Warning for anonymous users |
@hamza221 I meant something like this:
In this case we could also store the data to the database instead of local storage (see your second update) with the benefit of being independent of the browser. However, this is of course more work as we will have to adjust the API/backend as well. @susnux @jancborchardt @jotoeri What do you think? |
I would prefer to go with you suggested solution but only for logged-in users
|
Saving in DB is possible, but might be quite complex. As it sounds a bit like the "allow users to edit their answers" feature. Nevertheless I think we should go with @hamza221 PR here for the moment and just do not save for anonymous users. |
Yeah, it's better to have localStorage saving now for error prevention than work longer to get it into the database. The main case is accidentally closing tabs or browser crashes. :) |
|
Edit: custom messages are unsupported on all major browsers |
9abbf5d
to
7f51e3b
Compare
@hamza221 So as far as I can see you already implemented that we will only store and restore the answers to/from the LocalStorage if a user is logged in. This would fix the original issue and so for me it's fine. We can try to combine it with the leave message in a follow-up PR.
This could be implemented in I will do the code review and testing today :) Thank you very much! |
I'm not sure how . Which modal would you use. I don't think confirm() or alert () are adequate for this use case and I'm not sure how to make a custom modal blocking. |
All fine :) Let's move to a follow-up 👍🏻 |
7f51e3b
to
8c047b6
Compare
Signed-off-by: hamza221 <[email protected]>
8c047b6
to
b506c4a
Compare
@hamza221 Could you please have another look at the code: I missed a problem when you reload a form with saved values: the UI doesn't update when you choose an option of a checkbox/radio question... |
Created a new issue for the problem: #1780 |
closes #456