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

added options for auto redirect/beep on session timeout. #68

Closed
wants to merge 4 commits into from
Closed

added options for auto redirect/beep on session timeout. #68

wants to merge 4 commits into from

Conversation

tejas-LXIX
Copy link

Added the option to enable the user to choose whether he wants to be redirected to the homepage automatically after session timeout or not. Redirects to the "enter phone number" page autoRedirectCheckbox is checked. Otherwise, gives a "confirm" box to the user if autoRedirect set to 0.
Also,enabled the option for the user to select whether he wants a session timeout notification beep or not.
I have tested it myself and it seems to work properly.

@tejas-LXIX
Copy link
Author

Solves the issues described here. #57

@sushrut111
Copy link
Owner

Before I check, can you please merge main branch into yours?
I have updated some logic for beep.

Also, I was thinking it would be a lot to add any more inputs in the form. From the feedback I received, it is already confusing some people.

We could have something like advanced options and by default have the voice notification enabled which tells that session is expired. The advanced options will be visible only when they click on it and for now let keep the auto logout checkbox in advanced options.

What do you think @prayuj ?

@tejas-LXIX
Copy link
Author

My main branch is concurrent with yours. I have just made a few changes in the beep logic which make sure that the beep plays repeatedly if the user refuses to log in again.

Yes, that's a good idea. Keeping it in advanced options would reduce the clutter and make it easire to use.

@sushrut111
Copy link
Owner

I see that you changed the audio file path that I had kept. I have removed beep if you haven't already checked.

@tejas-LXIX
Copy link
Author

Okay I have corrected that. Anything else?

@sushrut111
Copy link
Owner

I will review this tomorrow. I hope you have considered the things I suggested in the comments above.

@tejas-LXIX
Copy link
Author

Yes yes, I have considered your points. It might take me some time to work on them though as I have exams coming up.

@sushrut111
Copy link
Owner

No worries. Let's see when we can tale it forward later. All the best for your exams!!

@prayuj
Copy link
Collaborator

prayuj commented May 31, 2021

Yes I agree, we can place those check boxes in a collapsible "Advanced Options" which users can then configure if needed. Also could you rename your branch, I am not able to checkout to it!
@sushrut111 @tejas-LXIX

@sushrut111
Copy link
Owner

sushrut111 commented May 31, 2021

@prayuj you can do

git fetch origin pull/68/head:pr68

on my repo.
A new branch will be created with name pr68 and you can check it out.

@prayuj
Copy link
Collaborator

prayuj commented May 31, 2021

@prayuj you can do

git fetch origin pull/68/head:pr68

on my repo.
A new branch will be created with name pr68 and you can check it out.

Yup that worked, thanks!

@sushrut111 sushrut111 closed this Jun 8, 2021
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

3 participants