-
Notifications
You must be signed in to change notification settings - Fork 27
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
Move ICS Feed button from Ilios Calendar to Main Navigation #7811
Move ICS Feed button from Ilios Calendar to Main Navigation #7811
Conversation
I wasn't able to figure out how to test the new tooltip notification upon clicking the ICS Feed button, but I removed some testing code that was no longer relevant. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think putting this here was my idea, but now that it's here I don't love it. It feels like too prominent a location for this and doesn't balance particularly well with the other buttons. Some discussion needed for sure. Maybe in the profile menu?
The tooltip copy works, but I think we have some better options. Take a look at:
- The copy button on a virtual learning URL in an offering
- The copy button on the API key generator at the bottom of the user profile page
I didn't look too deeply at the code as is, think we need to work out some UX first.
The API key copy notification is nice, yes, and could be used instead. As for the placement of the icon itself, I don't have a strong feeling one way or another. It looks like the user profile menu is pretty bare and could easily incorporate more functionality. However, would users know to look there for this? |
All righty: I changed the button to have an IliosTooltip on mouse hover, and the click success triggers a flash message instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you have the tooltip pop out to the right instead of underneath when there is space? If this ends up being more than an hour's work never mind. I'd also like to see the button switch to a check after it is clicked along with the confirmation.
Moved the tooltip so it displays to the right. If there's no room, then it falls back to bottom. As for the icon itself turning into a checkmark, you mean you want the copy button to go from RSS Feed (pre-click)->Checkmark (clicked)->RSS Feed (post-click)? |
Code looks good, assigning to @dartajax for another visual pass and merge. |
percy won't let me merge this one either but quick question - it looks like the .ics feed button is snuggling up to the Calendar button - they are related but is this visual representation intentional? I am cool with it just wondering |
we need to be aware - other support people as well as me - that when a user clicks Calendar, the screen shifts and the top menu is not viewable meaning that they may not be able to find the .ics feed button since it has been moved to the top of the screen - just pointing this out |
@dartajax The visual representation is intentional per ilios/ilios#4116 (comment). |
@dartajax This is a good point. I'm not sure how often people use this icon link, so not sure if it matters, though. @jrjohnson and @stopfstedt, thoughts? |
…url and instructions variables
…ios-tooltip for success notification
…st as it's contained in tooltip now
9b787ce
to
8b7e10f
Compare
My $.02 is that most students grab their feed from here so a lot of people use it. But we are in summer right now and new students can have a new interface and updated instructions on grabbing their feeds. |
I think the scrolling display is a different issue, we don't need to consider it here. Percy has now run, just needs someone to check and make sure all the changes are valid. |
Fixes ilios/ilios#4116.
The old functionality is to click on Calendar in the main navigation, click the ICS Feed button, and then click the "Copy My ICS Link" button in the flyout to copy the URL to the user's clipboard.
The new functionality is to click the ICS Feed button to copy the URL to the user's clipboard, directly from the main navigation, always available, and without the flyout.
Tests for this are still being figured out, so this is a draft for now.