-
-
Notifications
You must be signed in to change notification settings - Fork 362
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 box #5442
feat: notification box #5442
Conversation
✅ Deploy Preview for koda-canary ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
it's always ask here on the issue to stay on context, I believe not all of them are always around Discord than Github, for example my case 😁 |
hmmmm, just talked with @roiLeo seems the notification need external service? also currently i just displayed all events for the account. and not sure how to get the |
it's fine, we are doing local version first to give creators overview on their sales, we have right now none 😅
will be in future.
oh I'm afraid for that we will need to have resolver? |
IMO, an external database is needed to tell user which notification he has seen/deleted
issue reported in kodadot/snek#169 I don't see any limit in your queries, it might be a problem on client side |
Hey was talking w/ @yangwao that there are higher prio issues than - kodadot/snek#169 |
deployment is here https://deploy-preview-5442--koda-canary.netlify.app/ notification icon seems very dark |
seems the icon got lost, will fix it later |
AI-Generated Summary: This pull request adds a Notification Box feature to the Navbar. It introduces new components, such as |
hmm, tooltip, not sure if we should include that - I would add some delay, lets try 1s for now and then we can customize it later
sidebars still stack on top of each other |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now white line is shown on wallet sidebar not sure if it's related (#5552)
Tested on /snek/gallery/331660682-7
, I can see 4 "offer" events in NotificationBox but only 2 row in Offers tab maybe remove expired?
return colorMap[key] | ||
} | ||
|
||
export const useNotification = (account: string) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function useNotification
has 46 lines of code (exceeds 25 allowed). Consider refactoring.
AI-Generated Summary: This pull request introduces a notification system for users. It adds a |
@import '@/styles/abstracts/variables'; | ||
|
||
.notify-item { | ||
padding: 0.75rem 2rem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use bulma classes please
@daiagi hi dude thanks for your review! |
AI-Generated Summary: This pull request adds a notification box feature to the navbar component, allowing users to view and filter notifications related to their account. It includes the addition of a NotificationBoxButton component, a NotificationBoxModal component, and new GraphQL queries for fetching notifications data. Additionally, it adds new translations for the notification text and updates some styling variables. |
Hi I know it seems like a lot but it's small tasks each |
alright will do the changes. but seems bulma only support these 6 spacing not including like
|
AI-Generated Summary: This pull request introduces a new notification feature for the navbar. It adds a notification box button component to the navbar, which when clicked, displays a notification modal with a list of events, such as sales and offers, filtered by collections and event types. Users will now be able to view notifications related to their account inside the application. The code includes new GraphQL queries to fetch the required notification data and new components for displaying the notifications. Additionally, some new styles and utility classes for the notification feature have been added. |
Code Climate has analyzed commit 24098fb and detected 1 issue on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
AI-Generated Summary: This pull request introduces a new notification system for the application. It includes:
The new features provide users with the ability to view and manage notifications related to sales, offers, and accepted offers based on the selected filters. |
seems works fine for me
I have idea if you @daiagi you could handover this? As I really move forward this one :) On one PR is already quite a lot of effort and I want to avoid big PRs like this. |
pay 100 usd |
😍 Perfect, I’ve sent the payout 🪅 Let’s grab another issue and get rewarded! |
Thank you for your contribution to the KodaDot NFT gallery.
this pr is not completed yet, and have few questions:
events
query do not supportinteraction = offer / accepted offer
. or use another query?should notifications need ashould we limit the notifications on this? like display the latest 10?mark as read
like other apps? if so, i think we need a new backend api.just tried to contact some kodadev guys on discord for their advice, but seems they are offline these days and not get feedback from them :(
👇 _ Let's make a quick check before the contribution.
PR Type
Context
Before submitting pull request, please make sure:
Optional
Had issue bounty label?
Community participation
Screenshot 📸