-
-
Notifications
You must be signed in to change notification settings - Fork 17
Dex 1435 new notifications to fe #463
Changes from all commits
82f6bb1
b792610
633b655
b4e2c20
043edb1
82cfbd1
d66d30e
d0f5ef0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import React from 'react'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All of these new dialogs are essentially duplicates with different names. I still don't love this pattern, because it's much DRYer to just have a baseNotificationDialog component do this job, but Ben long ago recommended to do it this way in case the notifications needed to diverge farther from one another in the future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks to me like notifications have already diverged pretty far from each other, but instead of putting those differences in these files, we put them in the NotificationDetailsDialog. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah... NotificationDetailsDialog might be a great example of the abstraction monster situation described in https://www.deconstructconf.com/2019/dan-abramov-the-wet-codebase. Hoping to get the chance to make this better/more human-friendly in the future (but not for current ticket). |
||
import { get } from 'lodash-es'; | ||
import { notificationSchema } from '../../../constants/notificationSchema'; | ||
import NotificationDetailsDialog from '../NotificationDetailsDialog'; | ||
|
||
export default function NotificationCollaborationEditDeniedDialog({ | ||
notification, | ||
onClose, | ||
open, | ||
}) { | ||
const notificationType = notification?.message_type; | ||
const currentNotificationSchema = get( | ||
notificationSchema, | ||
notificationType, | ||
); | ||
const availableButtons = []; | ||
return ( | ||
<NotificationDetailsDialog | ||
open={open} | ||
onClose={onClose} | ||
notification={notification} | ||
titleId={get(currentNotificationSchema, 'titleId')} | ||
moreDetailedDescription={get( | ||
currentNotificationSchema, | ||
'moreDetailedDescription', | ||
)} | ||
buttons={availableButtons} | ||
/> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import React from 'react'; | ||
import { get } from 'lodash-es'; | ||
import { notificationSchema } from '../../../constants/notificationSchema'; | ||
import NotificationDetailsDialog from '../NotificationDetailsDialog'; | ||
|
||
export default function NotificationCollaborationManagerDeniedDialog({ | ||
notification, | ||
onClose, | ||
open, | ||
}) { | ||
const notificationType = notification?.message_type; | ||
const currentNotificationSchema = get( | ||
notificationSchema, | ||
notificationType, | ||
); | ||
const availableButtons = []; | ||
return ( | ||
<NotificationDetailsDialog | ||
open={open} | ||
onClose={onClose} | ||
notification={notification} | ||
titleId={get(currentNotificationSchema, 'titleId')} | ||
moreDetailedDescription={get( | ||
currentNotificationSchema, | ||
'moreDetailedDescription', | ||
)} | ||
buttons={availableButtons} | ||
/> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import React from 'react'; | ||
import { get } from 'lodash-es'; | ||
import { notificationSchema } from '../../../constants/notificationSchema'; | ||
import NotificationDetailsDialog from '../NotificationDetailsDialog'; | ||
|
||
export default function NotificationCollaborationManagerEditApprovedDialog({ | ||
notification, | ||
onClose, | ||
open, | ||
}) { | ||
const notificationType = notification?.message_type; | ||
const currentNotificationSchema = get( | ||
notificationSchema, | ||
notificationType, | ||
); | ||
const availableButtons = []; | ||
return ( | ||
<NotificationDetailsDialog | ||
open={open} | ||
onClose={onClose} | ||
notification={notification} | ||
titleId={get(currentNotificationSchema, 'titleId')} | ||
moreDetailedDescription={get( | ||
currentNotificationSchema, | ||
'moreDetailedDescription', | ||
)} | ||
buttons={availableButtons} | ||
/> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import React from 'react'; | ||
import { get } from 'lodash-es'; | ||
import { notificationSchema } from '../../../constants/notificationSchema'; | ||
import NotificationDetailsDialog from '../NotificationDetailsDialog'; | ||
|
||
export default function NotificationCollaborationManagerEditDeniedDialog({ | ||
notification, | ||
onClose, | ||
open, | ||
}) { | ||
const notificationType = notification?.message_type; | ||
const currentNotificationSchema = get( | ||
notificationSchema, | ||
notificationType, | ||
); | ||
const availableButtons = []; | ||
return ( | ||
<NotificationDetailsDialog | ||
open={open} | ||
onClose={onClose} | ||
notification={notification} | ||
titleId={get(currentNotificationSchema, 'titleId')} | ||
moreDetailedDescription={get( | ||
currentNotificationSchema, | ||
'moreDetailedDescription', | ||
)} | ||
buttons={availableButtons} | ||
/> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import React from 'react'; | ||
import { get } from 'lodash-es'; | ||
import { notificationSchema } from '../../../constants/notificationSchema'; | ||
import NotificationDetailsDialog from '../NotificationDetailsDialog'; | ||
|
||
export default function NotificationCollaborationManagerEditRevokeDialog({ | ||
notification, | ||
onClose, | ||
open, | ||
}) { | ||
const notificationType = notification?.message_type; | ||
const currentNotificationSchema = get( | ||
notificationSchema, | ||
notificationType, | ||
); | ||
const availableButtons = []; | ||
return ( | ||
<NotificationDetailsDialog | ||
open={open} | ||
onClose={onClose} | ||
notification={notification} | ||
titleId={get(currentNotificationSchema, 'titleId')} | ||
moreDetailedDescription={get( | ||
currentNotificationSchema, | ||
'moreDetailedDescription', | ||
)} | ||
buttons={availableButtons} | ||
/> | ||
); | ||
} |
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.
I fluctuated a little bit about whether I wanted this to be a prop here or just derive it with an api call in the child component.
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.
I ran into the same question on #464. I passed it as a prop too. It looks like React Query recommends just deriving it though.
TanStack/query#463
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.
I kept it as a prop for this ticket...
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.
That seems like a good idea to me. It is something we can look into and try to implement going forward.