-
Notifications
You must be signed in to change notification settings - Fork 9
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(Notifications): Added archived state, NotificationsPopupWrapper
component and enhanced the stories for notifications
#83
Conversation
…he Notifications' stories
…feat-added-archived-state-and-enhanced-the-stories-for-notifications
…ies and README.md
Preview is ready. |
NotificationsPopupWrapper
component and enhanced the stories for notificationsNotificationsPopupWrapper
component and enhanced the stories for notifications
const style = wrapperMaxHeight === undefined ? {} : {maxHeight: `${wrapperMaxHeight}px`}; | ||
|
||
if (isRemoved) { | ||
return <></>; |
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.
return null;
?
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.
Changed to null
…feat-added-archived-state-and-enhanced-the-stories-for-notifications
const [wrapperMaxHeight, setWrapperMaxHeight] = React.useState<number | undefined>(undefined); | ||
const [isRemoved, setIsRemoved] = React.useState(false); | ||
|
||
React.useEffect(() => { |
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.
it is a little bit weird that notification manages it's visibility state by itself
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.
It's because we need to animate the removing effect (archive
)
I thought it would be more convenient if we manage it in the NotificationWrapper
component
@korvin89 ping |
…feat-added-archived-state-and-enhanced-the-stories-for-notifications
Added features:
InfiniteScroll
functionality to Notifications.NotificationsPopupWrapper
component with some styles so it would be easier to use Notifications inside a popup.Fixed problems:
cursor: pointer
to active notifications (with non-emptyonClick
).[UXRFC-114]