Preferences notifications #821
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Notifications are now in same order as old mpdx
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
Contributor
Author
|
This PR is dependant on this PR https://github.com/CruGlobal/mpdx_api/pull/2677 |
Added disabled flags to checkboxes if submitting.
GraphQL should begin with captial letter
dr-bizz
commented
Nov 16, 2023
dr-bizz
commented
Nov 16, 2023
dr-bizz
commented
Nov 16, 2023
canac
requested changes
Nov 16, 2023
Contributor
canac
left a comment
canac
left a comment
There was a problem hiding this comment.
I was only able to look at the code so far. I'll try running it locally later today.
pages/accountLists/[accountListId]/settings/notifications.page.tsx
Outdated
Show resolved
Hide resolved
src/components/Settings/notifications/NotificationsTable.test.tsx
Outdated
Show resolved
Hide resolved
src/components/Settings/notifications/NotificationsTable.test.tsx
Outdated
Show resolved
Hide resolved
canac
reviewed
Nov 16, 2023
canac
reviewed
Nov 16, 2023
Contributor
|
It works great when I run it locally! The skeleton table while loading is a nice touch. |
canac
requested changes
Nov 21, 2023
Contributor
canac
left a comment
canac
left a comment
There was a problem hiding this comment.
A couple more tweaks and this should be good to go!
pages/accountLists/[accountListId]/settings/notifications.page.tsx
Outdated
Show resolved
Hide resolved
src/components/Settings/notifications/NotificationsTable.test.tsx
Outdated
Show resolved
Hide resolved
src/components/Settings/notifications/NotificationsTable.test.tsx
Outdated
Show resolved
Hide resolved
Caching notificationConstants after first load. Changing .reduce to .map
canac
reviewed
Dec 4, 2023
canac
approved these changes
Dec 4, 2023
Contributor
canac
left a comment
canac
left a comment
There was a problem hiding this comment.
Approving, pending two minor tweaks.
dr-bizz
added
the
On Staging
…ions and removed fetching constants and now fetching notification types
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Similar to #817, We've done a lot of work on Preferences. To prevent a 300-plus page PR I'm pushing certain changes into
mainwhich doesn't affect the user.This PR is the notifications page in Preferences.
Changes
Connect ServicesPR.)Checklist: