notifications-ui - New feature#9368
Conversation
|
I bet they'll introduce a popup soon similar to what we have in the PR Files tab. Anyway, worth the change now since I suppose they won't be working on notifications before December. |
|
Keep an eye on mobile too. I think our controls can appear on a single line so this would be improved already: Long screenshot
|
unwrap-unnecessary-dropdowns - Use segmented controlsnotifications-ui - New feature
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
| - [](# "pr-notification-link") [Points PR notifications to the Conversation tabs instead of the commits page, which may be a 404.](https://github.com/refined-github/refined-github/assets/1402241/621f6512-655e-4565-a37b-2b159ea0ffce) | ||
| - [](# "sticky-notifications-actions") [Make the notifications action bar sticky.](https://github.com/refined-github/refined-github/assets/1402241/5b370430-2319-4c78-88e7-c2c06cd1c30f) | ||
| - [](# "clean-notifications") Makes the notifications list more compact when grouped by repo. | ||
| - [](# "notifications-ui") [Transforms sorting and grouping into segmented controls.](https://github.com/user-attachments/assets/684c237f-65eb-41b7-aebf-3b624b665481) |
There was a problem hiding this comment.
Since we already have clean-notifications, maybe notifications-ui is too broad as a name. notifications-header? clean-notifications-header? Not too long or else it's impossible to mention it in PR titles
There was a problem hiding this comment.
Maybe quick-notifications or notifications-actions to keep it limited to unwrapping of:
- sorting
- grouping
notifications-ui- Unwrap "mark all as read/unread" dropdown #5704
And then yet another feature:
mobile-notifications: improves the layout of the notifications page header on mobile specifically
There was a problem hiding this comment.
clean-* features usually remove something. quick-notifications is as general as notifications-ui
There was a problem hiding this comment.
notifications-uiis as general asquick-notifications
I think it's more generic. "quick" implies it saves you a click, which is what this does
There was a problem hiding this comment.
unwrap-notifications-dropdowns?
The issue was changed to include a mobile UI improvements, so I unlinked it. By the way, what does it look like on mobile with this PR? |
|
What doesn't this PR cover? "Manage notifications" dropdown? |
Other than the two controls at the bottom, this still looks like crap 😃 
This is why I was suggesting creating two feature:
It's not a big deal to just have one It's often easier to reason when "a whole feature is broken/missing" rather than "this part of the feature in this specific scenario". This is also why I took |
Nothing to unwrap there, it has a bunch of options inside |
|
I'd rather merge this and then think about the rest later, this PR is already ready, we just need to settle on the name. |
|
|
CSS implementations of |
|
OK, maybe you're right. Reverted |
This reverts commit 9fb6ec3.
9814b4c to
04b2545
Compare
|
Looks much better already, however the native "Inbox | Gear" setup is actually "correct" since the modal actually goes to edit what appears in the Inbox. It's not actually a filter on the current view, it is settings. For this reason I don’t think we can do much about the icons and labels, they're both "settings". Technically the "Change filters" button should be inside the "Inbox" dropdown, but absolutely not worth changing it here.
|
|
I think just placing them next to the All/Unread toggle is a big improvement: 
On medium resolutions, the "Manage notifications" dropdown can be moved, unchanged, to the right side of the first line. This can be done via CSS alone. |
part of #9204
Resolves #9203
Resolves #9202
Test URLs
https://github.com/notifications
Screenshot
Other screenshots
Mobile layout: