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

Provide the user with an option to resume the test or not. #72

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

Conversation

abelparayil
Copy link

@abelparayil abelparayil commented Jan 30, 2023

Related to #70

Describe the changes you've made

  • Created a button that lets users to pause the test
  • On click a modal opens that blurs the test in the background
  • Also the modal has a resume button to continue with the test
  • Made changes to the javascript file to measure the time accurately with the pause function implement

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 did a local machine test and I have tested whether the correct time is getting displayed or not with some console logs too.

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.

Screenshots (Images)

Before the changes After the Changes
Screenshot 2023-01-30 at 4 56 42 PM Screenshot 2023-01-30 at 4 53 47 PM
  • On pausing

Screenshot 2023-01-30 at 4 59 49 PM

@netlify
Copy link

netlify bot commented Jan 30, 2023

Deploy Preview for arito ready!

Name Link
🔨 Latest commit bac5edb
🔍 Latest deploy log https://app.netlify.com/sites/arito/deploys/63d7cd88b27842000b7687c2
😎 Deploy Preview https://deploy-preview-72--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.

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

Can we also mute or pause the background music when paused? @abelparayil

@abelparayil
Copy link
Author

Yeah that is a good idea but, won't the user reduce the volume if they're not interested in background music?

@prakhartiwari0 prakhartiwari0 mentioned this pull request Feb 1, 2023
@prakhartiwari0
Copy link
Owner

@abelparayil Yeah that is an option, but I don't think that user will know that the music won't stop if they pause. And that is not good, we need to pause/mute the music while the test is paused.

@abelparayil
Copy link
Author

Ok no probs, I'll work on that too.

@abelparayil
Copy link
Author

Hey @prakhartiwari0 I have got a bit busy right now with class work. Maybe you can add the muting audio function as a new issue for someone else to work on with.

@prakhartiwari0
Copy link
Owner

@abelparayil There are some merge conflicts happening, can you please work on them and resolve them? Also if would be great if you could solve the mute audio issue too in this PR only, as it doesn't make sense to have a separate issue just for that, I think it will not be a big task.

@abelparayil
Copy link
Author

Yeah sure @prakhartiwari0! I'll work it out during this weekend.

@prakhartiwari0
Copy link
Owner

We have merged a PR @abelparayil, I am sorry but can you please once again solve the merge conflicts that are there now?

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.

None yet

2 participants