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

UP-3717 - session timeout #392

Merged

Conversation

jhelmer-unicon
Copy link
Contributor

No description provided.

… JS is not yet completed, but I think most of the basic backend work is good (except for universality).
…otstrap since bootstrap didn't work well in universality. Misc code cleanups.
@apetro apetro changed the title UP 3717 - session timeout UP-3717 - session timeout Jul 25, 2014
@apetro
Copy link
Member

apetro commented Jul 25, 2014

Handy link to the JIRA issue.

@@ -30,15 +30,16 @@
+-->
<folder ID="s2" hidden="true" immutable="true" name="Page Top folder" type="page-top" unremovable="true">
<channel fname="dynamic-respondr-skin" unremovable="false" hidden="false" immutable="false" ID="n3"/>
<channel fname="fragment-admin-exit" unremovable="false" hidden="false" immutable="false" ID="n4"/>
<channel fname="fragment-admin-exit" unremovable="false" hidden="false" immutable="false" ID="n5"/>
Copy link
Member

Choose a reason for hiding this comment

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

What's going on here with changing the node ID of fragment-admin-exit from n4 to n5? And more generally with changing node identifiers throughout this file?

Choose a reason for hiding this comment

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

As an unsolicited side note, there has been a trend in uPortal to keep the numbers sequential. I have found it annoying and contributing to a lot of noise. Maybe I should propose that we adopt a strategy of having the folder IDs 10 or 20 apart so we can add portlets without renumbering the file. I have better things to do with my time that manually reorganize IDs and I'm sure others too 😄 I'll throw that suggestion out to the dev list.

BTW Andrew aren't you supposed to be on vacation!!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops. I originally had the element in a different location and forgot to reset the numbers. Will fix.

Copy link
Member

Choose a reason for hiding this comment

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

UP-4190 now tracks adopting a sparse out-of-the-box node identifier convention.

</preference>
<preference>
<name>dialogDisplaySeconds</name>
<value>60</value>
Copy link
Member

Choose a reason for hiding this comment

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

Worth a comment here explaining the meaning of this setting right where an adopter would be setting it? Something like

<!-- The session timeout warning will start displaying this many seconds before 
       the end of the user's session and will remain on the screen for this many seconds 
       (until the end of the session) 
       at which point the portlet will redirect to the logout URL specified in logoutURLFragment. -->

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. Will add.

…ment to portlet.xml and changed "extend my session" button text to "Yes, keep me signed in" which is hopefully a bit more user-friendly.
@jhelmer-unicon
Copy link
Contributor Author

Added comment based on apetro's suggestion. Changed wording for "extend my session" button to "Yes, keep me signed in" which should be a bit more user-friendly.

apetro added a commit that referenced this pull request Aug 4, 2014
UP-3717 - Add client-side warning re session timeout

with opportunity to renew session.
@apetro apetro merged commit 45a3bff into uPortal-Project:master Aug 4, 2014
@jhelmer-unicon jhelmer-unicon deleted the UP-3717_session_timeout branch October 4, 2014 07:27
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.

3 participants