Skip to content

refactor(icons): use standard octicons #885

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

Merged
merged 2 commits into from
Mar 20, 2024

Conversation

setchy
Copy link
Member

@setchy setchy commented Mar 15, 2024

Replaces the refresh, settings, logout and add account icons with those from the octicons primer design system used extensively throughout our experience.

Note: if we can find a suitable replacement for the Quit icon from https://primer.style/foundations/icons then we'll be 100% octicon powered.

Light Dark
Before Screenshot 2024-03-15 at 7 21 30 AM Screenshot 2024-03-15 at 7 21 38 AM
After Screenshot 2024-03-19 at 2 49 50 AM Screenshot 2024-03-19 at 2 49 38 AM

@bmulholland
Copy link
Collaborator

For the Quit icon, how about just an X? I know we use it elsewhere, but it's pretty standard for "close this app" and is in Octicons.

@setchy
Copy link
Member Author

setchy commented Mar 18, 2024

For the Quit icon, how about just an X? I know we use it elsewhere, but it's pretty standard for "close this app" and is in Octicons.

updated screenshots using XCircleFill as the Quit icon

@afonsojramos
Copy link
Member

afonsojramos commented Mar 18, 2024

I think that grouping these changes all in one PR truly gave us a new perspective in what the changes would entail.

From a UI cohesiveness perspective, they seem as if they don't belong, mostly the ones in the bottom bar.
The size of the bottom bar also seems to have grown, but that seems like a simple mistake.

Food for thought: Maybe one alternative would be to go away with the bottom bar and adding a profile image with a dropdown with these three options, where the icons would be accompanied with a descriptive text. What do you think?

@afonsojramos
Copy link
Member

I think that grouping these changes all in one PR truly gave us a new perspective in what the changes would entail.

From a UI cohesiveness perspective, they seem as if they don't belong, mostly the ones in the bottom bar. The size of the bottom bar also seems to have grown, but that seems like a simple mistake.

Food for thought: Maybe one alternative would be to go away with the bottom bar and adding a profile image with a dropdown with these three options, where the icons would be accompanied with a descriptive text. What do you think?

Rephrased for the sake of being more clear about the issues I see with the current change.

@setchy
Copy link
Member Author

setchy commented Mar 18, 2024

The size of the bottom bar also seems to have grown, but that seems like a simple mistake.

yes, i had noticed similar. adjusted

@setchy
Copy link
Member Author

setchy commented Mar 18, 2024

Food for thought: Maybe one alternative would be to go away with the bottom bar and adding a profile image with a dropdown with these three options, where the icons would be accompanied with a descriptive text. What do you think?

i'm not a huge fan of putting these an additional click away (behind a dropdown)

@setchy
Copy link
Member Author

setchy commented Mar 18, 2024

From a UI cohesiveness perspective, they seem as if they don't belong, mostly the ones in the bottom bar.

Personal taste perhaps. 🤷

imho, this switch to fully adopt Octicons / Primer Design System makes Gitify feel more consistent and unified 🙃

Copy link
Member

@afonsojramos afonsojramos left a comment

Choose a reason for hiding this comment

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

If I am the minority here, I am open to conceding. But in that case, I think I'd switch for the no fill X, since we're not using Fill anywhere else 🧐

@setchy setchy requested a review from afonsojramos March 19, 2024 06:50
@setchy setchy force-pushed the feature/octicons branch from 4ad0005 to c42b53c Compare March 19, 2024 08:56
@afonsojramos afonsojramos merged commit 6224f68 into gitify-app:main Mar 20, 2024
9 checks passed
adufr pushed a commit to adufr/gitify that referenced this pull request Mar 25, 2024
@setchy setchy deleted the feature/octicons branch April 7, 2024 11:42
@setchy setchy added this to the Release 5.0.0 milestone Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactoring of existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants