775 account page notifications #825
Conversation
ekraffmiller
moved this from In Progress 💻
to Ready for Review ⏩
in IQSS Dataverse Project
|
|
||
| export function useNeedsUpdate() { | ||
| return useSyncExternalStore(needsUpdateStore.subscribe, needsUpdateStore.getSnapshot) | ||
| } |
There was a problem hiding this comment.
What about including the needsUpdateStore.ts logic in this same file?
Just saying to avoid one more extra file..
| const [notifications, setNotifications] = useState<Notification[]>([]) | ||
| const [isLoading, setIsLoading] = useState(true) | ||
| const [error, setError] = useState<string | null>(null) | ||
| const { user } = useContext(SessionContext) |
There was a problem hiding this comment.
We have a hook to access session context -> const { user } = useSession()
|
|
||
| getUnreadNotificationsCount(): Promise<number> { | ||
| return getUnreadNotificationsCount.execute().then((count) => { | ||
| console.log('Unread notifications count:', count) |
package.json
Outdated
| "i18next-browser-languagedetector": "7.0.1", | ||
| "i18next-http-backend": "2.1.1", | ||
| "js-md5": "0.8.3", | ||
| "lodash-es": "^4.17.21", |
There was a problem hiding this comment.
could you remove the caret? to install fixed dependencies versions following the same way as other dependencies you need to run:
npm i --save --save-exact lodash-es.
Same for @types, but those should be npm i -D --save --save-exact @types/lodash (-D from devDependency)
|
Thanks for the review @g-saracca! I made the suggested changes. I have to keep the PR version of js-dataverse because it is relying on a PR image of dataverse that hasn't been reviewed yet. So maybe best to keep it waiting until the PRs it depends on are ready. |
|
Thanks for addressing the changes ellen, approving PR! |
github-project-automation
bot
moved this from In Review 🔎
to Ready for QA ⏩
in IQSS Dataverse Project
|
merge conflicts :( |
What this PR does / why we need it:
Implements the Notifications tab of the Account Page.
Which issue(s) this PR closes:
Special notes for your reviewer:
This implementation uses a NotificationContext, and polling to get notification updates. I didn't add the icons next to the messages because I don't think they add much value, but if it's wanted I can add them.
Note that a
Suggestions on how to test this:
Do some actions in the SPA to trigger notifications. For example, create a new account, publish a dataset, upload a .csv file, or assign a role. The actions should update the Header to show the new unread notifications. Go to the Notifications tab to view them.
Note that a follow up PR will introduce paging, so that will handle the performance issues currently seen in JSF when there is a large amount of notifications.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Yes, rather than showing new notifications highlighted until the page is refreshed, as done in JSF, the SPA version of the page removes the highlight of the new notifications after a 2 second delay. After two seconds, the new notifications are marked as read, and the highlights are removed, and also the unread notifications badge is removed from the Header.
JSF Version
SPA Version
Is there a release notes update needed for this change?:
Additional documentation: