Skip to content

Conversation

hughlilly
Copy link
Collaborator

@hughlilly hughlilly commented Sep 21, 2022

Changes the function that is called when the GitHub icon is clicked. Since the Gitify “G” icon already goes to the Gitify repo, this button was, in effect, a duplicate. It makes sense to provide a way to easily open the GitHub notifications page when there are no new notifications.

Closes #543.

Opens GitHub notifcations page instead of Gitify repo (which can be opened by clicking the app's icon)
@hughlilly hughlilly marked this pull request as ready for review September 21, 2022 23:53
Copy link
Collaborator

@JakeSidSmith JakeSidSmith left a comment

Choose a reason for hiding this comment

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

LGTM

@afonsojramos
Copy link
Member

afonsojramos commented May 24, 2023

This doesn't fully make sense. I agree that the link should no longer go to gitify's repo, but is going to notifications logical? If you have notifications you already have the bell icon... It is still an improvement however!

@afonsojramos afonsojramos changed the title Open GitHub notifications page when clicking on GitHub icon fix: open GitHub notifications page when clicking on GitHub icon May 24, 2023
@afonsojramos
Copy link
Member

If you're up for it, there are some tests to fix. Otherwise, I'll fix them myself ;)

@hughlilly
Copy link
Collaborator Author

This doesn't fully make sense. I agree that the link should no longer go to gitify's repo, but is going to notifications logical? If you have notifications you already have the bell icon... It is still an improvement however!

Oh I didn’t realise the bell icon went to notifications! I’d argue if there are no notifications it’s still useful to have the bell icon there, but grey, perhaps?

@afonsojramos
Copy link
Member

Sounds good! Then I believe we can completely remove the GitHub icon. I see no need for it and see no other usage that would remove the duplication.

@afonsojramos
Copy link
Member

Closing this PR in favour of #587.

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.

The G logo should go to the configured git host home page
3 participants