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: notification filters #1750

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Soxasora
Copy link
Member

@Soxasora Soxasora commented Dec 20, 2024

Description

Adds filtering by type for Notifications completing #1035

Types are represented by categories that are defined and ordered in lib/constants.js

  • Replies: Reply, Mention, ItemMention
  • Followed: FollowActivity
  • Stacking: Votification, ForwardedVotification
  • Earnings: Earn, Revenue, ReferralReward
  • Territories: TerritoryPost, TerritoryTransfer, SubStatus
  • Reminders: Reminder
  • Referral: Invitification, Referral
  • Payments: Invoice, InvoicePaid, WithdrawlPaid
  • Streak: Streak

Like Satistics, it relies on router.query to apply and retrieve filters (a user can bookmark the URL).
Unlike it, the modal enables multi-select before applying filters and it uses router.replace instead of router.push to maintain interoperability with the existing notifications page

When filters are applied, localStorage is updated with values set by the user and/or router.query.inc. If filters haven't been reset, on next visits users can re-apply the last filters they chose

Screenshots

Notifications page
image

Default modal, no filters
image

Modal with filters
image

Mobile modal
image

Notifications page with filters on
Screenshot 2024-12-20 at 14 45 33

Additional Context

  • SubStatus.name as id and casting NULL earnedSats values to BIGINT was necessary to make queries work on their own, could this be a problem? Notifications works as usual :P

  • Also I couldn't find a query for JobChanged so I left it as-is

  • Filters hue for light mode maybe needs to be changed?

image
  • FilterIcon color could be green or other color, I used --theme-brandColor

Checklist

Are your changes backwards compatible? Please answer below: Yes!

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
7, I couldn't reproduce some notification types! Other than that, filtering consistently works and when no filters are applied, notifications follow the same behavior as before.

For frontend changes: Tested on mobile, light and dark mode? Please answer below: Tested on mobile, PWA and also in light (additional context) and dark mode.

Did you introduce any new environment variables? If so, call them out explicitly here: No

Progress

  • better UI
  • localStorage implementation

@Soxasora Soxasora marked this pull request as ready for review December 21, 2024 11:55
@Soxasora Soxasora marked this pull request as draft December 22, 2024 15:20
@Soxasora Soxasora marked this pull request as ready for review December 22, 2024 16:36
@Soxasora Soxasora added the feature new product features that weren't there before label Dec 29, 2024
Copy link
Member

@huumn huumn left a comment

Choose a reason for hiding this comment

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

Code looks simple and clean. I'll QA and do UI review tomorrow.

Copy link
Member

@huumn huumn left a comment

Choose a reason for hiding this comment

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

This is great.

I won't merge until after Jan. 3, and I might end up tweaking the UI and save the notification filters to localstoage for reuse, but consider it done - I'll pay up in the next few days.

@Soxasora
Copy link
Member Author

I knew I was forgetting something, filter persistence!

I could go and do it right away^^
Thanks a lot k00b!

@Soxasora Soxasora marked this pull request as draft January 7, 2025 12:17
@Soxasora Soxasora force-pushed the notification_filters branch from 825a566 to 0c1b64b Compare January 8, 2025 09:10
@Soxasora Soxasora force-pushed the notification_filters branch from 188aa17 to dfa3e07 Compare January 9, 2025 15:40
@Soxasora Soxasora force-pushed the notification_filters branch from dfa3e07 to 7a8ed9b Compare January 9, 2025 15:41
@Soxasora Soxasora force-pushed the notification_filters branch from 81fc06f to 05e7057 Compare January 9, 2025 16:25
@Soxasora Soxasora marked this pull request as ready for review January 9, 2025 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature new product features that weren't there before
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants