Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

MemberPresenceAvatar: fix null references #1620

Merged
merged 4 commits into from
Nov 21, 2017
Merged

MemberPresenceAvatar: fix null references #1620

merged 4 commits into from
Nov 21, 2017

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Nov 17, 2017

avoid assuming that member has a user

@richvdh
MemberPresenceAvatar: fix null references
4e7fd55 
avoid assuming that `member` has a `user`
@turt2live
Copy link
Member

Although this fixes the error, I'm not convinced it fixes the issue. The render function also assumes it gets some kind of value for both of those. It should probably not show a presence indicator (or the option to change presence) if there's no presence information.

@turt2live
Disable presence controls if there's no presence
28ff0a4 
Signed-off-by: Travis Ralston <travpc@gmail.com>
@turt2live turt2live mentioned this pull request Nov 18, 2017
Merged
@turt2live
Copy link
Member

turt2live commented Nov 18, 2017 •
edited
Loading

@richvdh I've pulled in your changes to see what happens, and it appears as though it effectively disables the icon but leaves the context menu accessible. #1623 should make sure that the handler is also ignored. Feel free to reauthor or redirect the PR: I've targeted against this PR's branch.

The combination of these PRs fixes element-hq/element-web#5629

@richvdh
Copy link
Member Author

richvdh commented Nov 21, 2017

@turt2live : is there a way to enable this? under labs I only have 'Message Pinning'.

@turt2live
Copy link
Member

@richvdh yes, but riot.im/develop has a configuration problem (still). The feature is feature_presence_management not feature_presence.

turt2live and others added 2 commits November 21, 2017 09:01
@turt2live
Update MemberPresenceAvatar.js
5f3fda2 
Signed-off-by: Travis Ralston <travpc@gmail.com>
@richvdh
Merge pull request #1623 from turt2live/travis/presence-break
91dbe95 
Disable presence controls if there's no presence
@richvdh richvdh merged commit b2e80a8 into develop Nov 21, 2017
turt2live added a commit to turt2live/matrix-react-sdk that referenced this pull request Dec 25, 2017
@turt2live
Remove presence management
cc5eb77 
The feature is incredibly buggy and doesn't work as expected due to server behaviour and client interaction. One of the major problems is the constantly confused presence state - this is caused by the mobile apps conflicting on the state of the web app, causing it to consider the user offline or online (and rarely away) depending on how riot-android/ios is behaving at the time.

This reverts two PRs:
* matrix-org#1620
* matrix-org#1482

The changes to the context menu positioning were not reverted as they are useful outside of presence management.

Signed-off-by: Travis Ralston <travpc@gmail.com>
@turt2live turt2live mentioned this pull request Dec 25, 2017
Merged
dbkr pushed a commit that referenced this pull request Apr 12, 2018
@turt2live @dbkr
Remove presence management
91a4139 
The feature is incredibly buggy and doesn't work as expected due to server behaviour and client interaction. One of the major problems is the constantly confused presence state - this is caused by the mobile apps conflicting on the state of the web app, causing it to consider the user offline or online (and rarely away) depending on how riot-android/ios is behaving at the time.

This reverts two PRs:
* #1620
* #1482

The changes to the context menu positioning were not reverted as they are useful outside of presence management.

Signed-off-by: Travis Ralston <travpc@gmail.com>
@richvdh richvdh deleted the rav/fix_memberpresence_avatar branch November 30, 2022 10:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@richvdh @turt2live