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: show notification number for issues, prs, discussions #1276

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

setchy
Copy link
Member

@setchy setchy commented Jun 19, 2024

Updated
Screenshot 2024-06-21 at 7 17 43 AM

Original
Screenshot 2024-06-19 at 9 49 02 AM

@setchy setchy added the enhancement New feature or enhancement to existing functionality label Jun 19, 2024
@setchy setchy added this to the Release 5.10.0 milestone Jun 19, 2024
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.

I'd leave this one marinating a bit... I'm not sure if this is useful information and it is polluting an already quite polluted UI.

@afonsojramos
Copy link
Member

Maybe reducing opacity to like 60? And making it a font step smaller?

@setchy
Copy link
Member Author

setchy commented Jun 19, 2024

I'd leave this one marinating a bit... I'm not sure if this is useful information and it is polluting an already quite polluted UI.

It's another step on our journey to GitHub UI/UX consistency.

It wasn't until this week that I realized nowhere in our current UX do we mention the issue numbers 🙃

@setchy
Copy link
Member Author

setchy commented Jun 19, 2024

And making it a font step smaller?

I have already set it as xs instead of sm. Are you suggesting it should be even smaller?

fwiw, this is it mocked up using our xxs size

Screenshot 2024-06-19 at 11 20 47 AM

@setchy
Copy link
Member Author

setchy commented Jun 19, 2024

Maybe reducing opacity to like 60?

Now with updated opacity to 60

Screenshot 2024-06-19 at 11 17 19 AM

@setchy
Copy link
Member Author

setchy commented Jun 19, 2024

An alternate idea, we could model this as a another pill or something along these lines

Screenshot 2024-06-19 at 11 23 21 AM

@setchy
Copy link
Member Author

setchy commented Jun 19, 2024

Another option, adding it below the notification type icon (it's rough, needs proper formatting)

Screenshot 2024-06-19 at 11 39 00 AM

@afonsojramos
Copy link
Member

It's another step on our journey to GitHub UI/UX consistency.

I agree with the part about consistency, but we must have less UI elements, because we don't have the luxury of a full page imo

@setchy
Copy link
Member Author

setchy commented Jun 19, 2024

Maybe reducing opacity to like 60?

Now with updated opacity to 60

Screenshot 2024-06-19 at 11 17 19 AM

This remains as my preferred approach out of the three options above

@afonsojramos
Copy link
Member

afonsojramos commented Jun 20, 2024

My preferred approach is no issue number at the end of the day. I've locally played around with xxs and opacity-50, but it all comes down to this: what is the use of the issue number? As something that should focus on the essential, this is definitely not essential. I'd say this even goes against the following non-goal that we defined in #655:

Specific UX/UI changes that add options and/or visual complexity for minor workflow improvements.

Edit: Now also in https://github.com/gitify-app/gitify/blob/main/CONTRIBUTING.md#project-philosophy

@afonsojramos
Copy link
Member

@bmulholland @setchy @Araxeus @adufr opinions on the above?

@Araxeus
Copy link
Contributor

Araxeus commented Jun 20, 2024

I personally don't see value in showing issue number in Gitify, but I guess it could be optional?

@setchy
Copy link
Member Author

setchy commented Jun 20, 2024

I still think this is pretty innocent change 😅

using this mockup;

  • for short titles, you'll see the number as you do in the other GitHub Notification UIs
  • for long titles it will overflow off the screen and only be visible on notification mouse-over

If it's such a big concern, another option could be only having it in the mouse-over label text 🤷‍♂️

@afonsojramos
Copy link
Member

afonsojramos commented Jun 20, 2024

If it's such a big concern, another option could be only having it in the mouse-over label text 🤷‍♂️

Point still remains, what is the value of it? But if this is useful to anyone, that is indeed a good compromise!

@setchy
Copy link
Member Author

setchy commented Jun 20, 2024

I personally use the Issue/PR numbers a lot with the GH CLI

For example
Screenshot 2024-06-20 at 1 51 38 PM

@Araxeus
Copy link
Contributor

Araxeus commented Jun 20, 2024

I personally use the Issue/PR numbers a lot with the GH CLI

Would you really check out a branch based on a number from a notification title?

I think you would usually enter the notification, see what's actually inside it, then decide if you should check out or not

@adufr
Copy link
Contributor

adufr commented Jun 21, 2024

I'll give my opinion on this since I've been tagged, even though I haven't contributed recently

I like the fact that Gitify can be modular. I personally don't have a use case for this, but IMO it's worth adding it behind a setting (probably disabled by default though?)

we don't have the luxury of a full page imo

@afonsojramos: you are very right, that's why I'd disable it by default, but having multiple features like this that could be enabled/disabled would solve this problem; the user can just add whatever elements he wants and which fits its use-case

@setchy: about the design, I think I prefer this one too 😁
image

@setchy
Copy link
Member Author

setchy commented Jun 21, 2024

Realized I had been sharing a incorrect draft. Updated with the latest version

Screenshot 2024-06-21 at 7 17 43 AM

@afonsojramos
Copy link
Member

afonsojramos commented Jun 25, 2024

@afonsojramos: you are very right, that's why I'd disable it by default, but having multiple features like this that could be enabled/disabled would solve this problem; the user can just add whatever elements he wants and which fits its use-case

The issue with "the user can just add whatever elements he wants and which fits its use-case", is that then we'll need to have this in mind with future developments, as they are things that we might not be seeing in local development, but might break or affect without us realising. Too many options can be very hard to work with.

But, this is minimal I suppose, so I think we can move forward with this 🚀 (However, we'll need another setting for this I reckon)

@setchy
Copy link
Member Author

setchy commented Jun 25, 2024

However, we'll need another setting for this I reckon

the issue/pr/discussion number is already flagged with the detailed notification setting 🙂

@adufr adufr mentioned this pull request Jun 25, 2024
@afonsojramos
Copy link
Member

the issue/pr/discussion number is already flagged with the detailed notification setting 🙂

@setchy So do colors, last commenter and state, which I think are very useful and with no extra UI elements, while this one does add a new element, hence the ask for a new setting.

@setchy
Copy link
Member Author

setchy commented Jun 25, 2024

Added a new setting.

@setchy setchy marked this pull request as draft June 25, 2024 23:42
@setchy setchy marked this pull request as ready for review June 26, 2024 02:07
@setchy setchy requested a review from afonsojramos June 26, 2024 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or enhancement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants