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

feat: Add appropriate entities to showcase modal notifications #226

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jonathanmtran
Copy link
Contributor

@jonathanmtran jonathanmtran commented Oct 4, 2018

Checklist
Description of change

https://github.com/Jasig/NotificationPortlet/blob/master/notification-portlet-webapp/docs/modal.md

TODO
  • Use the Notification modal Web Component (not the portlet)
  • Apply the notification to Students only
  • Dismissing the modal window lasts the entire session
  • Better text than 'Lorem ipsum'

@drewwills
Copy link
Contributor

Folks,

This PR is a very welcome sight -- it was on my personal TODO list.

Some changes I think we should make:

  • Use the Notification modal Web Component (not the portlet)
  • Better text than 'Lorem ipsum'
  • Apply the notification to Students only
  • Dismissing the modal window lasts the entire session

@jonathanmtran
Copy link
Contributor Author

jonathanmtran commented Oct 4, 2018

  • Use the Notification modal Web Component (not the portlet)
  • Apply the notification to Students only
  • Dismissing the modal window lasts the entire session

Sure thing

  • Better text than 'Lorem ipsum'

What did you have in mind @drewwills ?

@drewwills
Copy link
Contributor

What did you have in mind @drewwills ?

I'll take a stab...

Student Portal Updates

Welcome to the new student portal based on <a href="https://apereo.org/projects/uPortal">uPortal 5</a>. There are several exciting new features included with this update. Please take a moment to familiarize yourself with the new content & options.

…odal

Includes updated modal notification (replaces lorem ipsum)
@jonathanmtran
Copy link
Contributor Author

This is where c23c1ae puts us

screenshot from 2018-10-04 13-34-46

Had to use NotificationPortlet-4.2.0 as NotificationPortlet-4.3.x was throwing erroring with a regenerator runtime error

Also, the modal doesn't render in Firefox

@jonathanmtran
Copy link
Contributor Author

  • Dismissing the modal window lasts the entire session

uPortal-Project/NotificationPortlet#114 appears to be what we would need to make this happen. Is that right?

@drewwills
Copy link
Contributor

That's correct @jonathanmtran. I have code (from a uP implementation project) that's about a 95% match. I think @bjagg also has some very similar code. One of us can probably post a PR for that feature soonish.

@bjagg
Copy link
Member

bjagg commented Oct 5, 2018

Yes, I can have a PR in a few days for uPortal-Project/NotificationPortlet#114

@ChristianMurphy
Copy link
Member

@bjagg is uPortal-Project/NotificationPortlet#114 still on the roadmap?

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

Successfully merging this pull request may close these issues.

4 participants