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

Implement user-controlled presence #1482

Merged
merged 11 commits into from
Nov 14, 2017
Merged

Conversation

Includes rudimentary support for custom statuses and user-controlled status. Some minor tweaks have also been made to better control how we advertise our presence.

Signed-off-by: Travis Ralston <travpc@gmail.com>
Signed-off-by: Travis Ralston <travpc@gmail.com>
Signed-off-by: Travis Ralston <travpc@gmail.com>
Signed-off-by: Travis Ralston <travpc@gmail.com>
Signed-off-by: Travis Ralston <travpc@gmail.com>
Signed-off-by: Travis Ralston <travpc@gmail.com>
Signed-off-by: Travis Ralston <travpc@gmail.com>
Signed-off-by: Travis Ralston <travpc@gmail.com>
Signed-off-by: Travis Ralston <travpc@gmail.com>
@turt2live turt2live changed the title [WIP] Implement user-controlled presence Implement user-controlled presence Oct 25, 2017
@turt2live
Copy link
Member Author

This is ready for review. I can't figure out the UX for custom away statuses in a sensible way, so I'm pushing that off to a later PR (potentially by someone else).

@ara4n
Copy link
Member

ara4n commented Nov 14, 2017

hm, this is another one of these massive features that somehow I've totally missed the PR for - sorry :( we seem to be rather stretched on folks to check PRs atm.

@ara4n
Copy link
Member

ara4n commented Nov 14, 2017

I'm slightly surprised that Synapse actually supports this, as I don't think anyone has ever actually used the custom presence support (although it was indeed implemented in the outset)...

Presence.stopMaintainingStatus();
if (newStatus === "online") {
Presence.setState(newStatus);
} else Presence.setState(newStatus, null, true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

surprised the linter doesn't complain bitterly about this - the codestyle is to avoid oneliner if & else blocks please :)

@ara4n
Copy link
Member

ara4n commented Nov 14, 2017

So this looks excellent :) Only thoughts are:

  • Technically the 'official' vector designs reserve use of the MessageComposer avatar to switch accounts or personae once multiaccount is implemented. However, I've always had massive reservations of how this would work in practice - after all, a given personae isn't necessarily in the current room, and using it to switch the whole app into a different account would feel very strange. So nowadays I think that the final Groups UX will provide a much better way of handling multiple personae & accounts, and instead it's fine to use this for presence (a bit like MSN did, iirc?)
  • Having set yourself as 'away', how does that actually appear in the MemberList to people? I would expect to see some matching MemberInfo or MemberList changes?

Otherwise this looks good to merge modulo minor style nitpick.

@turt2live
Copy link
Member Author

I saw those designs somewhere, and I would agree the account picker is better suited elsewhere (possibly user settings post-tabbed-settings?). The MSN style behaviour is an intentional choice to mimic other applications, like Discord.

The users show up as "Away" in the RHS. Much like "Idle", the user is 50% opacity.

@ara4n
Copy link
Member

ara4n commented Nov 14, 2017

cool. again, i'm amazed that MemberInfo does the right thing - particularly as bits of the presence codebase try to use the terms 'away' and 'idle' interchangeably(!).

lgtm - thanks!

@ara4n ara4n merged commit f05958e into matrix-org:develop Nov 14, 2017
turt2live added a commit to turt2live/matrix-react-sdk that referenced this pull request Dec 25, 2017
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>
dbkr pushed a commit that referenced this pull request Apr 12, 2018
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>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants