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

ui: session - clicking outside the Session Timeout Warning Dialog closes the dialog without handling "Stay connected" #1561

Closed
SuperITMan opened this issue Jan 21, 2020 · 12 comments · Fixed by #1605

Comments

@SuperITMan
Copy link
Member

SuperITMan commented Jan 21, 2020

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[X] Bug report  
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/NationalBankBelgium/stark/blob/master/CONTRIBUTING.md#got-a-question-or-problem

Current behavior

When the Session Timeout Warning Dialog appears

image

And you click outside the dialog, it closes the dialog without handling the "Stay connected".

EDIT: If you click outside of the dialog, it closes it and does not stop the countdown. That means that you are disconnected once the countdown is finished. That's really bad for the UX 😕

Expected behavior

2 behaviours are possible here:

  1. When clicking outside the dialog, nothing happens
  2. When clicking outside the dialog, the "Stay connected" behaviour is triggered.

Minimal reproduction of the problem with instructions

Open the showcase, wait for the Session Timeout Warning Dialog appearing then click outside the dialog.

What is the motivation / use case for changing the behavior?

Environment


Angular version: 7.x
Stark version: 10.0.0-rc.3


@SuperITMan SuperITMan added this to the 10.0.0-rc.4 milestone Jan 21, 2020
@SuperITMan SuperITMan added this to To do in 10.0.0-rc.4 via automation Jan 21, 2020
@SuperITMan
Copy link
Member Author

@nicanac @christophercr Which behaviour do you prefer ?

Personally I prefer the second one, clicking outside should make the dialog disappear.

@christophercr
Copy link
Collaborator

Mmm I think I prefer option 1: clicking outside does nothing. That way the user is forced to click on the "Stay connected" button if he really wishes to keep his session alive (that also feels like kind of safer right?).

Otherwise, triggering the "stay connected" behavior when clicking outside might be confusing... because if the app displays such dialog it is for a reason and the user is expected to interact with it

@nicanac
Copy link
Contributor

nicanac commented Jan 22, 2020

At first, option 2 was my favorite because in most case like in a picture viewer, when you click outside it closes the popup.
But, I agree with you @christophercr , we are in the case of a connection, a bit like if it was a login.
To avoid any confusion, I've the feeling we have to force the user to at least read the button :-)

@SuperITMan
Copy link
Member Author

Okok. I agree.
In that case, if the user wants to close his session at that time, how can he do it ? 😊
I suggest to add a button "Disconnect". What do you think ?

@nicanac
Copy link
Contributor

nicanac commented Jan 22, 2020

Okok. I agree.
In that case, if the user wants to close his session at that time, how can he do it ? 😊
I suggest to add a button "Disconnect". What do you think ?

Yes, good idea.

@christophercr
Copy link
Collaborator

Yep, good idea... but is it really something urgently requested by anyone?

@SuperITMan
Copy link
Member Author

@christophercr
Actually, it's not a matter of implementing a new feature but of fixing a bug.
I edited my issue to explain better the current behaviour which is bad.

@christophercr
Copy link
Collaborator

christophercr commented Jan 22, 2020

Ah ok, well, in that case the solution is simple: just add the option disableClose in this line:

.open<StarkSessionTimeoutWarningDialogComponent>(StarkSessionTimeoutWarningDialogComponent, { data: action.countdown })

And about the Disconnect button, I don't think is really necessary since the user will be logged out anyways (just some X seconds later). That said, I'm not against about such "nice to have" either 😉

@SuperITMan
Copy link
Member Author

Yep'! Exactly. It's a simple fix :-) (that should be part of 10.0.0-rc.4 I think)
Maybe we can create another issue/PR for the "Disconnect" button. What do you think ? 😊

@christophercr
Copy link
Collaborator

Alright! Then let's create another issue/PR for it 😉

@SuperITMan SuperITMan modified the milestones: 10.0.0-rc.4, 10.0.0 Feb 6, 2020
@SuperITMan SuperITMan removed this from To do in 10.0.0-rc.4 Feb 6, 2020
@SuperITMan SuperITMan added this to To do in 10.0.0 via automation Feb 6, 2020
@SuperITMan SuperITMan modified the milestones: 10.0.0, 10.0.1 Mar 10, 2020
@christophercr christophercr added this to To do in 10.0.1 via automation Mar 16, 2020
@christophercr christophercr removed this from To do in 10.0.0 Mar 16, 2020
@christophercr christophercr modified the milestones: 10.0.1, 10.1.0 Mar 19, 2020
@christophercr
Copy link
Collaborator

I've created issue #1731 for the implementation of the "Disconnect" button 😉

@christophercr christophercr modified the milestones: 10.1.0, 10.0.0-rc.4 Mar 20, 2020
@christophercr christophercr removed this from To do in 10.0.1 Mar 20, 2020
@christophercr
Copy link
Collaborator

Closed by #1605

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants