Show an unread badge when there are unread notifs from other accounts #3398
Conversation
|
It turns out I've included an unwanted commit. Will open another PR... |
Reverted :D |
There was a problem hiding this comment.
- the check if "sync all accounts" is activated is missing, it is always shown, which does not make sense if only the active account is synced the unread count of the others can never change
- I would like to have the unread badge rather on the right in the sidebar and displaying the count of unread messages (the badge on the hamburger menu would be to small for a number, but in the sidebar is enough space for it)
- fix event listener leak
- move special animation css colors to themebase if we want them
- move update code into unread badge itself and have two variants (one small one for the hamburger menu and a bigger one with number for the sidebar.)
| accountIds.map(id => onDCEvent(id, 'IncomingMsg', update)) | ||
| ) | ||
|
|
||
| return update |
There was a problem hiding this comment.
the return value of useEffect should be a callback that removes the listeners again, that you created above
There was a problem hiding this comment.
Can you explain more please?
There was a problem hiding this comment.
https://react.dev/reference/react/useEffect#useeffect
the return value of the use effect function/callback must be a callback that does cleanup, in this case a callback that removes the event listeners again.
This reverts commit a16e1e8.
farooqkz
force-pushed
the
far-show-unread-multi-acc-badge
branch
from
094c590 to
2ebc6d6
Compare
|
@Simon-Laux I think I've solved all the problems except for the listener leak thing. I will be grateful if you do a suggestion so that I can learn how should I handle this as it's my first time using P.S.: I think the animation of the badge in the sidebar needs some tweaking. Perhaps no animation is better? Suggestions welcome! |
| @@ -181,7 +181,10 @@ const Sidebar = React.memo( | |||
| </div> | |||
| <div key='logout' className='sidebar-item' onClick={onLogout}> | |||
| {tx('switch_account')} | |||
| <OtherAccountsUnreadBadge top='-38px' left='-14px' /> | |||
| <OtherAccountsUnreadBadge | |||
| style={{ top: '-38px', left: '110px' }} | |||
There was a problem hiding this comment.
I would prefer having these offsets in they (s)css unless the are either for prototyping or dynamic.
There was a problem hiding this comment.
The thing is that the parent component has to determine the offsets according to its use. The offsets are different for the hamburger menu icon and in the sidebar.
There was a problem hiding this comment.
don't use global offsets if you can, in this case it does not make much sense to use global offsets. Also they can be styled in the scss file of the component they are used in/with.
|
Anyways I would postpone this as there is no clear consensus of the other dc team members if this feature is actually effective enough for it's intended use case. |
|
Not sure myself -- would need to use it for a while to know. |
It was purely my idea to enhance multi account usage. Wasn't discussed or requested in the forum. |
|
Let's rather go for the full sidebar solution soon. closing this for now |
This PR adds an unread badge next to hamburger icon and "switch account" button in side bar when there are unread notifications in other account(s).