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.

Sign up for GitHub

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

Jump to bottom

Saving progress when tab is refreshed or closed #53

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Saving progress when tab is refreshed or closed #53

wants to merge 9 commits into from

Conversation

Anan7Codes
Copy link

Related to #33

Describe the changes you've made

Saving the necessary data in local storage to save(persist) data(progress) upon refresh or if the tab is closed.

Type of change

What sort of change have you made:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, local variables)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

I've only tested it locally.

Checklist:

  • My code follows the guidelines of this project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly wherever it was hard to understand.
  • I have made corresponding changes to the documentation.

Test URL

https://arito.vercel.app/
(Will take the URL down once we sort it out and merge)

@netlify
Copy link

netlify bot commented Jan 3, 2023 •
edited

✅ Deploy Preview for arito ready!

Name Link
🔨 Latest commit 96d67ff
🔍 Latest deploy log https://app.netlify.com/sites/arito/deploys/63c673590fcdde000801cc8f
😎 Deploy Preview https://deploy-preview-53--arito.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

github-actions[bot]
github-actions bot reviewed Jan 3, 2023
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congratulations on your First Pull Request! Wish you best on your open-source journey 😊

@prakhartiwari0
Copy link
Owner

prakhartiwari0 commented Jan 5, 2023 •
edited

Hey @Anan7Codes, thanks for the PR, you have done an amazing job!
(Btw I don't know why you have hosted it on Vercel also, as you can see above Netlify automatically generates a deploy preview URL for all the PRs to check the changes live in the app, this is it - https://deploy-preview-53--arito.netlify.app/)

I will be responding very soon after checking if everything is working well and telling you if there are any bugs or other problems.

Happy Contributing!

@prakhartiwari0
Copy link
Owner

I have noticed a small bug, the time is getting reset when we reload the app (or close and open again) and the total time taken is actually being shown as the time taken to complete after the latest reloading (or reopening).

For example, let's say 10 seconds were spent on solving the questions, and then I reload the app and then take only 5 seconds to complete the test (because less number of questions were there to be solved) and the Final Results show 5 seconds, which actually should be 15 seconds, that is, all the time taken in the previous instances added.

I hope I was able to explain, I would like to know what are your thoughts and can you solve this bug or not.

@Anan7Codes
Copy link
Author

Anan7Codes commented Jan 5, 2023 •
edited

I have noticed a small bug, the time is getting reset when we reload the app (or close and open again) and the total time taken is actually being shown as the time taken to complete after the latest reloading (or reopening).

For example, let's say 10 seconds were spent on solving the questions, and then I reload the app and then take only 5 seconds to complete the test (because less number of questions were there to be solved) and the Final Results show 5 seconds, which actually should be 15 seconds, that is, all the time taken in the previous instances added.

I hope I was able to explain, I would like to know what are your thoughts and can you solve this bug or not.

Yeah, I didn't notice the preview link my bad. I've taken the vercel link down.

I'll look into this issue and update you.

@Anan7Codes
Copy link
Author

I found the mistake.
I was saving the start_time but not making use of it. I think it works now, can you check? Thanks!

@prakhartiwari0
Copy link
Owner

Okay so now there is another bug I just now observed... @Anan7Codes

Let's suppose we are giving a test of 5 questions, and we take 10 secs to reach the final question, and then close the app. Now the issue is that the time is actually continued to be counted in the background (maybe because of the files that are stored locally still running) and when you open the app after, let's say 1 minute, and then solve the final question in 2 secs, it should actually show the total time taken as 12 seconds, but it shows 1 minute and 12 seconds!

@Anan7Codes
Copy link
Author

Anan7Codes commented Jan 5, 2023 •
edited

Okay so now there is another bug I just now observed... @Anan7Codes

Let's suppose we are giving a test of 5 questions, and we take 10 secs to reach the final question, and then close the app. Now the issue is that the time is actually continued to be counted in the background (maybe because of the files that are stored locally still running) and when you open the app after, let's say 1 minute, and then solve the final question in 2 secs, it should actually show the total time taken as 12 seconds, but it shows 1 minute and 12 seconds!

Yea I got what you are saying. So right now when the test page is created there is a start time and the end time is when the result page is created. So irrespective of whether the tab is closed or not, time is counted.

If we are only counting the time taken when the tab is open and the test is going on, then I think the approach would be to calculate the time taken to answer each question and add it at the end to generate the total time. This way we could show the time taken to answer individual questions as well I guess.

What do you think?

@prakhartiwari0
Copy link
Owner

prakhartiwari0 commented Jan 5, 2023 •
edited

Hey @Anan7Codes, what you are suggesting will be implemented in near future as "Average Time Taken for each question" in the Results Page. But as of now, we don't need that much detail.
And I don't think it would solve the problem, as then the time will start to be counted whenever a new question comes, and because the script is stored and run in the background, I think it will again lead to the same problem (however I may be wrong).

Whatever may happen, currently our aim is to solve the bug of extra time counted even after the app is closed, that is, stop the time counting when the app is closed, and then start it when the app is opened again (or reloaded) and add the time taken while giving the test (or say when the question page is open) only.

Forgive me for my complicated language and grammatical errors 😅

Was I able to explain?

@Anan7Codes
Copy link
Author

Hey @Anan7Codes, what you are suggesting will be implemented in near future as "Average Time Taken for each question" in the Results Page. But as of now, we don't need that much detail.

And I don't think it would solve the problem, as then the time will start to be counted whenever a new question comes, and because the script is stored and run in the background, I think it will again lead to the same problem (however I may be wrong).

Whatever may happen, currently our aim is to solve the bug of extra time counted even after the app is closed, that is, stop the time counting when the app is closed, and then start it when the app is opened again (or reloaded) and add the time taken while giving the test (or say when the question page is open) only.

Forgive me for my complicated language and grammatical errors 😅

Was I able to explain?

No worries. I got you clearly. But technically if they close the tab and come back maybe 2 hours later, shouldn't we still count the time since it is a test 🤔

@prakhartiwari0
Copy link
Owner

prakhartiwari0 commented Jan 5, 2023 •
edited

@Anan7Codes See, the thing is,
The only good reason I could think of a user closing or reloading is by mistake, and if it is by mistake, maybe the device got some problem or the internet is gone and they cannot open the app again. And in that scenario, nobody would want to count the time they were not giving the test.
And even if they do it deliberately, what's the point?
Not all the questions are revealed right away, you need to answer each question before proceeding to the next one.

Let's say they are too smart and they just put random values as answers and get to the end to have all the questions, and then close the app, solve all of them using a calculator or just take more time, and submit and get full marks!
Bro, apply common sense, what's the point of taking the test if you want to take more time or use a calculator, I mean why are you giving the test in the first place, to improve YOUR calculation, right? So basically this is a question of self-honesty, and the test giver will be the one who suffers and not us! Our job is to provide them with an app so that they can practice and improve, that's it. Self-improvement is only the self's job and not others.

LOL, I gave a whole damn lecture! Sorry for that but I think I was able to explain...

@Anan7Codes
Copy link
Author

@Anan7Codes See, the thing is, The only good reason I could think of a user closing or reloading is by mistake, and if it is by mistake, maybe the device got some problem or the internet is gone and they cannot open the app again. And in that scenario, nobody would want to count the time they were not giving the test. And even if they do it deliberately, what's the point? Not all the questions are revealed right away, you need to answer each question before proceeding to the next one.

Let's say they are too smart and they just put random values as answers and get to the end to have all the questions, and then close the app, solve all of them using a calculator or just take more time, and submit and get full marks! Bro, apply common sense, what's the point of taking the test if you want to take more time or use a calculator, I mean why are you giving the test in the first place, to improve YOUR calculation, right? So basically this is a question of self-honesty, and the test giver will be the one who suffers and not us! Our job is to provide them with an app so that they can practice and improve, that's it. Self-improvement is only the self's job and not others.

LOL, I gave a whole damn lecture! Sorry for that but I think I was able to explain...

lol fair enough. I'll make the changes and commit it.

@Anan7Codes
Copy link
Author

Done.

@Anan7Codes
Copy link
Author

There still remains a small issue. My next commit will fix it. I know what it is.

@Anan7Codes
Copy link
Author

Alright so right now,

  • It doesn't account for the time lost out of the tab by refresh or closure.
  • Time counts if the student goes back and re-answers a question.
  • 2nd point is obvious but with this update, it'll be easy to calculate the time taken per question individually since that is on your roadmap.
  • The start time for the latest question at which the tab is closed or refreshed, is reset when the student is back on the page.

@Anan7Codes
Copy link
Author

Alright so right now,

  • It doesn't account for the time lost out of the tab by refresh or closure.
  • Time counts if the student goes back and re-answers a question.
  • 2nd point is obvious but with this update, it'll be easy to calculate the time taken per question individually since that is on your roadmap.
  • The start time for the latest question at which the tab is closed or refreshed, is reset when the student is back on the page.

If you find this too complicated, the easier approach would be to just calculate the time when window.onbeforeunload is triggered and just add it to the next session. Let me know if you want to do it this way.

@prakhartiwari0
Copy link
Owner

I just checked and observed that the time is getting reset after we reload and give the test.

@prakhartiwari0
Copy link
Owner

Watch this once @Anan7Codes

recording.mp4

@prakhartiwari0
Copy link
Owner

The Total Time Taken should be ~28 secs more or less a bit. But it is nearly half of it!
I don't know if something is wrong in the code or if you are trying to do something else, kindly explain this phenomenon to clear my confusion @Anan7Codes

Thank you

@Anan7Codes
Copy link
Author

Anan7Codes commented Jan 5, 2023 •
edited

No phenomenon, mate. It was my bad.

A recording from my side for on refresh
https://user-images.githubusercontent.com/37017231/210799547-a6a47cdb-f101-45f4-a025-9b89701cdc14.mov

And this recording is when the tab is closed.
https://user-images.githubusercontent.com/37017231/210799622-4058956f-65d3-4e37-9db8-60d45ecb0e96.mov

@prakhartiwari0
Copy link
Owner

Hey @Anan7Codes
We will be merging this PR after adding some buttons for navigation and other functions, and then we will remove the prompt that comes before unloading.
There will be some merge conflicts, and therefore it will be good to tackle all of them after making adding those buttons

@prakhartiwari0
Copy link
Owner

Hey @Anan7Codes Can you please resolve the merge conflicts?

@prakhartiwari0
Copy link
Owner

prakhartiwari0 commented Jan 16, 2023 •
edited

Also, I was thinking will it better if we provide the user with an option to resume the test or not.
Because if the user doesn't want to resume the test, they will be able to discard it.

What do you think about this? @Anan7Codes
And can you implement this feature?

@Anan7Codes
Copy link
Author

I've recorded a few tests as well.

@Anan7Codes
Copy link
Author

Anan7Codes commented Jan 17, 2023 •
edited

Hey @Anan7Codes Can you please resolve the merge conflicts?

Done. Can you review it? I've added recordings from my side as well.

@Anan7Codes
Copy link
Author

Also, I was thinking will it better if we provide the user with an option to resume the test or not. Because if the user doesn't want to resume the test, they will be able to discard it.

What do you think about this? @Anan7Codes And can you implement this feature?

I think it's better as a new issue as it'll be a good opportunity for somebody else.

@prakhartiwari0
Copy link
Owner

I think it's better as a new issue as it'll be a good opportunity for somebody else.

Yeah, you are right, also, we will be adding some buttons like back, and exit, to provide the user with the options to exit a certain stage of the app, and not get stuck in that test, or starting test page.

@prakhartiwari0
Copy link
Owner

Hey @Anan7Codes Can you please resolve the merge conflicts?

Done. Can you review it? I've added recordings from my side as well.

Sure! Will take some time as I am a little busy these days, will inform you once I am done with it.

@abelparayil abelparayil mentioned this pull request Jan 26, 2023
Open
@prakhartiwari0 prakhartiwari0 mentioned this pull request Feb 1, 2023
Closed
@prakhartiwari0
Copy link
Owner

prakhartiwari0 commented Feb 21, 2023 •
edited

We will merge this PR after merging #72. A few merge conflicts will appear which we will have to resolve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Anan7Codes @prakhartiwari0