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

Implement news webhook to handle release announcements #12396

Merged
merged 6 commits into from Sep 8, 2021

Conversation

hexylena
Copy link
Member

@hexylena hexylena commented Sep 1, 2021

This is based off of the version of the Galaxy server, and the version last seen by the user (as reported by localStorage.) When an admin upgrades their server, the users of that server will see a notification icon that will hopefully draw their eyes to seeing the new features in their version of Galaxy.

fixes the comment in #3950 (comment) I guess. There's a notification icon. And a pip! And then it gets 'marked read'. Whee.

Peek 2021-09-01 16-07

Design Commentary

Discussed with @shiltemann the preferred icon, this bell seemed like a good choice because it would give us a place in the future to put notifications if we implement such a system. We could simply combine it into a multi-item news feed and it'd be fine for users.

I tested out a couple different icons, I wasn't super happy with any, but this seemed like it was OK + worked for the future.

For the pip, I thought about making it the active colour

Open Question

Should we do something about dev branch? Either opening the release notes early and updating them as we go along? Or just leave it?

I guess main users will see this as main updates before we publish our announcements? That really feels like motivation to make the release notes earlier and update them as we go along.

How to test the changes?

  • Instructions for manual testing are as follows:
    1. Open Galaxy (well, not dev, a version before dev since the release notes page doesn't exist for that.)
    2. A notification icon appears in the masthead, along with a little 'pip' I think they're called
    3. Clicking on it opens the release notes for that version of galaxy, and removes the pip
    4. The pip stays removed (assuming one doesn't clear session/etc.)

License

@bgruening
Copy link
Member

@hexylena
Copy link
Member Author

hexylena commented Sep 1, 2021

Ahh would you want to add that to #2324 @bgruening? My 5 year old issue for that feature could use those nice new use cases.

Edit, you already commented that there.

@bgruening
Copy link
Member

Edit, you already commented that there.

And I didn't know it anymore :)

@hexylena hexylena mentioned this pull request Sep 1, 2021
@dannon
Copy link
Member

dannon commented Sep 2, 2021

This is nice. I think I'd really like the whole webhook icon (not just the pip) to disappear after use, though? What do folks think?

I'm a little worried about crowding the masthead with things folks use very sparingly.

@hexylena
Copy link
Member Author

hexylena commented Sep 2, 2021

  • I definitely wanted the "no news" state to be unintrusive, I'm likewise worried it's "noisy" for something that you want rarely.
  • I worry that if it disappears, people who mis-click out of the release notes won't be able to find them again,
  • This has the future outlook of mutating into the notification system, which is something we'd want to be persistent? (But that's a weak argument, that something might happen in the future, depending on when we can find someone.)

It's too bad we seem to be on an older fontawesome version, we could replace it with the outline version or so, to further de-emphasize it, in addition to removing the pip

image

@nsoranzo
Copy link
Member

nsoranzo commented Sep 2, 2021

I'm a little worried about crowding the masthead with things folks use very sparingly.

  • I worry that if it disappears, people who mis-click out of the release notes won't be able to find them again,

I also think it makes sense to have a place where to go back to old notifications once they are read, assuming the notification system is going to be used for more that just the release announcements.

@dannon
Copy link
Member

dannon commented Sep 2, 2021

* I definitely wanted the "no news" state to be unintrusive, I'm likewise worried it's "noisy" for something that you want rarely.

Yep, that's my main concern here.

* I worry that if it disappears, people who mis-click out of the release notes won't be able to find them again,

They're linked directly in the help section right next to this, so we could also add text at the top of the popup saying "You can always find the latest release notes ? Or just open in a new tab instead of iframe if that is really likely to be an issue where people can't find them?

It's too bad we seem to be on an older fontawesome version, we could replace it with the outline version or so, to further de-emphasize it, in addition to removing the pip

Our fontawesome supports this just fine, I'll follow up with a commit here.

@hexylena
Copy link
Member Author

hexylena commented Sep 2, 2021 via email

@hexylena
Copy link
Member Author

hexylena commented Sep 2, 2021

Ahh had to remove the fa class, ok, that's what I was missing!

And thanks for the reformat, I ran it through prettier cd client ; prettier --write ../path I figured it'd load the prettifer config in that folder since that was how the makefile looked, but maybe not

@dannon
Copy link
Member

dannon commented Sep 2, 2021

@hexylena Yeah, that's reasonable. I think it's personal preference -- I generally want everything in separate tabs and I find the popup frames more disorienting, but the idea of potentially clicking on a new feature in the release notes and having it pop up or something is pretty compelling.

Regarding the dev branch it should just show the same release that the release notes dropdown shows, so the previous 21.05 I think. which is also busted. We should probably target the previous release here, instead of a dead page in both places?

@hexylena
Copy link
Member Author

hexylena commented Sep 3, 2021

We should probably target the previous release here, instead of a dead page in both places?

Any convenient way to get the 'previous release'? Or just checking for the 404 with a fetch you reckon?

That was part of my motivation for suggesting opening the release notes earlier, would nicely sidestep that :P

@hexylena
Copy link
Member Author

hexylena commented Sep 8, 2021

how's hexylena@36782f9 sound @dannon

@dannon
Copy link
Member

dannon commented Sep 8, 2021

@hexylena It'll work for primary servers for release, want to add a 21.01 -> 21.09 for dev, then we've got a couple months?

@hexylena
Copy link
Member Author

hexylena commented Sep 8, 2021

Smart

@dannon dannon merged commit 0fe1304 into galaxyproject:dev Sep 8, 2021
@dannon
Copy link
Member

dannon commented Sep 8, 2021

@hexylena Thanks, this is nice!

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.

None yet

4 participants