Away status #3263

borisyankov · 2019-01-10T06:19:30Z

Implements #3266

borisyankov · 2019-01-11T02:34:40Z

While the PR itself is WIP (waiting for clarification and discussion on some back-end functionality) all three current commits can be merged already.

gnprice · 2019-01-14T22:12:35Z

Thanks @borisyankov !

There are some rebase conflicts with the refactors to action and event types that I made last week. I think all of the actual conflicts will be easy to resolve.

One thing that would be great is in the commit that does this:

+export type EventUserStatusUpdateAction = {|
+  ...ServerEvent,
+  type: typeof EVENT_USER_STATUS_UPDATE,
+  away: boolean,
+  user_id: number,
+|};
+

let's use the new src/api/eventTypes.js to write down exactly what the server actually sends in these events. Then the action type can refer to that -- much like EventPresenceAction does now.

gnprice · 2019-01-14T22:14:07Z

(In general, whenever we're adding a new event type -- or for another reason doing the work of nailing down exactly what the server sends in a given type of event -- I'd like to use the new eventTypes.js to write that information down while it's fresh.)

gnprice · 2019-01-29T00:22:13Z

(It looks like my comments above about eventTypes.js still apply.)

gnprice · 2019-01-29T00:38:42Z

src/api/setAwayStatus.js

+};
+
+export default (auth: Auth, params: SetAwayStatusParams): Promise<ApiResponseSuccess> =>
+  apiPost(auth, 'users/me/status', objectToParams(params));


What's the reasoning for using objectToParams here?

I believe it should have no effect at all -- neither of these properties are permitted to be an array, or undefined. So it seems simpler to just say params.

The function is used for its ability to remove keys with undefined values.

This isn't much of a reply. I wrote above:

I believe it should have no effect at all -- neither of these properties are permitted to be an array, or undefined.

If that's mistaken, would you explain why? If not, why do you take steps to remove keys with undefined values?

Aha, I see in a later commit that you do pass it params objects that have undefined values. I was confused to see that Flow accepted that; it turns out that the documented semantics of ?: properties is like so:

In addition to their set value type, these optional properties can either be void or omitted altogether. However, they cannot be null.

So indeed this is doing something useful, then.

(This reply remains not much of a reply.)

borisyankov · 2019-01-31T15:33:59Z

This is ready for review.

gnprice · 2019-01-31T19:34:08Z

src/events/eventToAction.js

+      return {
+        ...event,
+        type: EVENT_USER_STATUS_UPDATE,
+      };


Take a look at the block of 8 event types up above, with actionTypeOfEventType. This should go there.

Oh, didn't notice these case refactorings.
I am not sure this is an improvement, removing simple duplication by increasing the complexity of the code...
Anyway, did the change.

Cool.

I am not sure this is an improvement, removing simple duplication by increasing the complexity of the code...

I think the main thing I liked about this particular small refactoring was that we had all these different cases which were doing essentially the same thing... but that wasn't obvious because they were scattered around this switch statement and interleaved with cases that worked differently. And so each of them added complexity as you had to look at each case individually to see what it did. Now it's very clear that these all have the exact same boring structure, and the exceptions which do something more interesting stand out better.

There are other ways that could be done. Simply reordering cases to bring these together would have accomplished that and certainly not added any complexity, and would have been fine though remained verbose. When I went to make it less verbose, I chose this particular style largely because... this is what the existing code in the file already does for a bunch of other cases! 🙂

gnprice · 2019-01-31T19:36:34Z

src/api/eventTypes.js

+  ...EventCommon,
+  user_id: number,
+  away?: boolean,
+  status_text?: string,


Huh interesting! Are both of these really optional?

Also: Is your description here based on empirical observation, or reading the server code, or discussions with Steve, or something else?

Based on all three. One or two of them exist. Never none (as expected).

OK, so, a thing that's really important to get here is an idea of what one of these events means.

What does it mean about a user when we get an event with away: true?

What if the previous event for them had a status_text, and now this one doesn't mention status_text -- does that mean they no longer have a status_text? Or does the old one stay in place?

It sounds like you have some idea of what these are supposed to mean! Otherwise it would make no sense at all to say something like "as expected". I really do not know what to expect, because I do not know what these events mean.

I can guess, basically from imagining if I were designing a system related to user away status, what design choices would lead to an interface that looked like this one... but there are multiple possibilities there, and Steve may reasonably have had another one in mind entirely.

It's good that you have some idea of what these are supposed to mean. It is super important that that idea get written down somewhere. I think a bit of jsdoc on this event type might be the ideal place for that.

Ah cool, I see that the UserStatus types in the app's own data structures now has some quite helpful jsdoc, and of course that overlaps with this information.

No need to duplicate info. Any given piece of information can be written down in one of the places where it's important, and then write "See also UserStatus in src/types.js" or whatever in each of the other places.

I think the key bit of info that that doesn't cover and is important for this event type is: what does it mean when one of these is missing? Also away: false, which can't occur in the other place.

(Ideally I think the information on that UserStatus type would be in api/ somewhere -- particularly as we seem to use it in InitialData, which is really mainly a matter of describing the API. But it's fine to defer doing that rearrangement.)

Indeed. I had a commit somewhere that I think I dropped because it was causing too many merge conflicts that moved InitialData and all related types to apiTypes.

gnprice · 2019-01-31T19:49:21Z

src/api/setAwayStatus.js

+};
+
+export default (auth: Auth, params: SetAwayStatusParams): Promise<ApiResponseSuccess> =>
+  apiPost(auth, 'users/me/status', objectToParams(params));


Aha, I see in a later commit that you do pass it params objects that have undefined values. I was confused to see that Flow accepted that; it turns out that the documented semantics of ?: properties is like so:

In addition to their set value type, these optional properties can either be void or omitted altogether. However, they cannot be null.

So indeed this is doing something useful, then.

(This reply remains not much of a reply.)

gnprice

Thanks @borisyankov for this update!

Looks good at a high level; comments on specifics below.

(I'm not quite done looking at the reducer, in the interesting case with EVENT_USER_STATUS_UPDATE; breaking now for lunch so sending what I have, but might have another thought or two on that bit later.)

src/users/usersActions.js

gnprice · 2019-01-31T19:54:33Z

src/api/eventTypes.js

@@ -110,6 +110,13 @@ export type PresenceEvent = {|
  presence: UserPresence,
 |};

+export type UserStatusEvent = {|


This also needs the type property -- that's a pretty important one 🙂

Cool, I see this is done in an updated version of the branch. Small thing there -- the pattern I've set in this EventTypes enum is for the names to exactly match the values:

static update_message_flags: 'update_message_flags' = 'update_message_flags';

I'd like to continue that pattern here, so EventTypes.user_status.

Updated, haven't noticed that.
In lights of this, I think this is just a way more verbose version of the type:
update_message_flags | user_status
which will also not allow us to specify an invalid value and hopefully will provide auto-suggest as well.

Hmm, I don't think I'm quite picturing what alternative code you have in mind. Want to send a small PR to demonstrate? Happy to experiment with other ways of writing this.

gnprice · 2019-01-31T19:56:29Z

src/types.js

@@ -706,6 +718,10 @@ export type InitialDataUpdateMessageFlags = {|
  },
 |};

+export type InitialDataUserStatus = {|
+  user_status: UserStatusMap,


The commit message for this says:

Pass `user_status` to `fetch_event_types` to receive user status data on initial load. The data itself is in `away_user_ids` array.

I don't see an away_user_ids, though. Was that perhaps a previous version of this API?

In any case, the commit message and the code should agree.

I did update the commit message and made the user_status value optional to reflect that it might not exist.

Thanks! I appreciate the jsdoc on the UserStatus type, too.

FWIW my inclination would be to leave the description of user_status out of the commit message, as it's covered quite nicely in the jsdoc.

gnprice · 2019-01-31T19:59:42Z

src/user-status/userStatusReducers.js

+} from '../actionConstants';
+import { NULL_OBJECT } from '../nullObjects';
+
+const initialState: UserStatusState = NULL_OBJECT;


This isn't a place where we benefit from the singleton-ness of NULL_OBJECT -- there's never a reason to ===-compare one of these states with some other unrelated object that might happen to be empty. So the simpler {} is preferable.

Completely disagree because:

we are currently using this pattern everywhere

there are comparisons happening, any selector that uses that or any component that uses that as props does a comparison against the old value

as a bonus this object is frozen and will throw if we mutate it, revealing a potential bug

there are comparisons happening, any selector that uses that or any component that uses that as props does a comparison against the old value

Right, but that's perfectly well served by const initialState: UserStatusState = {};.

(That's why I wrote "with some other unrelated object that might happen to be empty".)

If there's ever a place where we compare one of these with a value that comes from some other mention of NULL_OBJECT -- like some other reducer for a different part of the state tree -- that would indicate a very worrying bug.

we are currently using this pattern everywhere

Hmm, true:

$ perl -lne 'print $1 if (/^const initialState.* = (.*)/)' src/*/*Reducer*.js | sort | uniq -c 5 { 1 getStateForRoute('loading') || NULL_NAV_STATE; 12 NULL_ARRAY; 9 NULL_OBJECT;

So shrug I guess it's fine for this PR to go along with the pattern, then. I might go through at some point and see about changing them all together, though.

as a bonus this object is frozen and will throw if we mutate it, revealing a potential bug

This is certainly something that could be useful in general!

I am not sure there is much value, though, in only having it on the empty values. I'd rate it a lot more likely to catch bugs if we were to do it systematically, on all the objects and arrays in our Redux state trees. The Reselect cached selector values would also be a good target.

I suspect the operation on deep object is slow as Object.freeze() does not work deeply and the deepFreeze we use in the unit-tests is iterating children calling Object.freeze() on them if array or object.

It seems the source code for that is shorter than an explanation:
https://github.com/substack/deep-freeze/blob/master/index.js

It seems the source code for that is shorter than an explanation

Like so much of the JS library ecosystem. 😝 Thanks for the link!

Yeah, that implementation may not be a suitable one for normal use. I think a good version of this would probably have to look something like: freeze each object/array as we create it, and count on its property values having been respectively frozen when they were created, without trying to iterate through them to double-check that.

Then the one potentially tricky bit is finding a way to spell that that doesn't make all our reducers (and cached selectors) annoyingly verbose.

gnprice · 2019-01-31T20:00:57Z

src/directSelectors.js

@@ -54,6 +55,8 @@ export const getTopics = (state: GlobalState): TopicsState => state.topics;

 export const getUserGroups = (state: GlobalState): UserGroup[] => state.userGroups;

+export const getUserStatus = (state: GlobalState): UserStatusState => state.userStatus;


This should go in the same commit that adds this branch to the state tree.

gnprice · 2019-01-31T20:06:32Z

src/user-status/userStatusReducers.js

+      return initialState;
+
+    case REALM_INIT:
+      return action.data.user_status || initialState;


The commit message points out:

Note: Older back-ends do not have a `user_status` key and we handle this correctly by leaving the state in a valid, empty state.

Quite so! This is an important bit of logic to have.

As we've discussed recently, if it's important enough to have code for, it's important enough to have in the types. The type should look like user_status?: UserStatusMap. Ideally with a line of jsdoc saying what this commit message says: that older servers (older than what version?) don't send that property.

Thanks for adjusting the type!

I'd still like to see this part too:

Ideally with a line of jsdoc saying what this commit message says: that older servers (older than what version?) don't send that property.

I don't even know what the next version will be called and the current official version does not support it. ¯(ツ)/¯

OK but say what you do know! Even the fact that "older" here means "all existing releases as of today, but not master" is much more informative than just "older" -- just "older" could easily mean "until some release a year or two ago".

The point here is to not make someone in the future have to go do an archaeological investigation, when they just want to learn a fact that is obvious to you right now.

E.g.: "Older servers (up through at least 1.9.1) don't send this property."

src/user-status/userStatusReducers.js

gnprice

And those last few comments. Over to you!

gnprice · 2019-01-31T21:07:15Z

src/user-status/userStatusReducers.js

+      return {
+        ...state,
+        [action.user_id]: newUserStatus,
+      };


Hmm, this is all kind of awkwardly verbose.

How about we let the values in our state be undefined rather than absent? Then we can write something like

const status = state[action.user_id] || {}; return { ...state, [action.user_id]: { away: action.away !== undefined ? action.away : status.away, status_text: action.status_text !== undefined ? action.status_text : status.status_text, }, };

(I don't love that version either, but it feels a little more compact and direct.)

This code has some drawbacks.
If previously we hadn't set away or status_text it will now be set to undefined.

I am going in the other direction. The value away will never be false but will miss completely, status_text will never be null, undefined or an empty string with length=0 but will also be deleted.

If previously we hadn't set away or status_text it will now be set to undefined.

Well yes, that is what I said 🙂 :

How about we let the values in our state be undefined rather than absent?

More precisely, that would mean that each of those properties would always be set, whether to undefined or to something else.

I am going in the other direction. The value away will never be false but will miss completely, status_text will never be null, undefined or an empty string with length=0 but will also be deleted.

OK, cool -- I'll look at the resulting code shortly.

OK, now that I see the intended semantics for these (away has just two states, and status_text as empty-string is synonymous with missing), I think probably the cleanest thing would actually be to have a type like this in our Redux state tree: {| away: boolean, status_text: string |}. Then this code would look like

/** The status a user implicitly has when not present in the state. */ export const DEFAULT_STATUS = { away: false, status_text: '' }; // ... const status = state[action.user_id] || DEFAULT_STATUS; return { ...state, [action.user_id]: { away: action.away !== undefined ? action.away : status.away, status_text: action.status_text !== undefined ? action.status_text : status.status_text, }, };

One advantage of this would be that it makes explicit what it means when we don't have this kind of data on a given user -- unlike for many other kinds of data where it would mean "we don't know", here it means "they have these default values". The export is there for a selector to use to encapsulate that fact.

I'm OK merging a version with the existing design, though.

src/types.js

gnprice · 2019-02-05T04:48:03Z

src/types.js

+ *       * when missing the presence is not changed
+ *       * can not be `false`
+ * @prop status_text - a string representing information the user decided to
+ *   manually set as his 'current status'


(Thanks again for adding this jsdoc!)

A much more specific followup question, now that the overall semantics are laid out here:

Can status_text be present but undefined? What about ''? If either of those is possible, do they mean something different from being absent?

Aha, I see the commit message for the later commit "Handle 'EVENT_USER_STATUS_UPDATE' action" has this information!

Currently the web app does update the `away` value separately from the `status_text` string and that is the immediate plan for mobile too. It is possible though that both values are changed at once. * if an event contains `away: false` this has the meaning of 'clear away overriding' * if an event contains `status_text: ''` this means 'clear status text' There is value in keeping our internal state simpler and consistent with what the back-end gives us as values. To achieve this we make sure if we get `away: false` and `status_text: ''` we remove the corresponding keys.

So it sounds like the answer is:

can't be undefined, nor ''

instead, will just be absent.

This is essential info for understanding the code around this type. Not only the reducer which maintains it, but also code which consumes it. And -- key test for what belongs in the code + comments, and not only commit message -- it's not at all specific to the change between this commit and its parent, and instead is 100% relevant to understanding the new code from scratch as it is, and for any future version of the codebase where these bits of code looks more or less like as they do here.

So let's stick that in the jsdoc too.

(There's also some information in there answering my questions about the semantics of the events. The same thing applies.)

gnprice · 2019-02-05T04:53:18Z

src/types.js

 };

+/**
+ * Specifies user status related properties
+ * @prop away - present if we are to override user's presence status


This line strikes me as a good cue to add a "See also" pointing at where we discuss presence... and an even better cue to add a "See also" there pointing at here, because this feature is kind of action-at-a-distance upon presence and could be surprising if you're looking at some presence behavior and don't know to look at this area.

The UserPresence type looks like a good main entry point to point at. Looks like that's currently in api/eventTypes.js. (It probably doesn't ultimately belong quite there, but certainly somewhere under api/. Anyway, the "see also" only needs to mention it by name.)

gnprice · 2019-02-05T05:17:54Z

src/api/eventTypes.js

@@ -17,7 +17,7 @@ import type { Message, Stream } from './apiTypes';
 //

 /** See ClientPresence, and the doc linked there. */
-export type UserStatus = 'active' | 'idle' | 'offline';
+export type PresenceStatus = 'active' | 'idle' | 'offline';


This is a good rename! I think in fact it belongs at the very beginning of the branch -- then that properly clears things up for starting to use the term "user status" for this new feature.

Ditto the other related rename.

gnprice · 2019-02-05T05:18:57Z

src/user-status/userStatusReducers.js

+      return initialState;
+
+    case REALM_INIT:
+      return action.data.user_status || initialState;


Thanks for adjusting the type!

I'd still like to see this part too:

Ideally with a line of jsdoc saying what this commit message says: that older servers (older than what version?) don't send that property.

gnprice

Thanks @borisyankov ! Several comments below.

I plan to merge some of these commits shortly; will comment separately when I do.

gnprice · 2019-02-05T05:28:27Z

src/types.js

+ *       * when missing the presence is not changed
+ *       * can not be `false`
+ * @prop status_text - a string representing information the user decided to
+ *   manually set as his 'current status'


Aha, I see the commit message for the later commit "Handle 'EVENT_USER_STATUS_UPDATE' action" has this information!

Currently the web app does update the `away` value separately from the `status_text` string and that is the immediate plan for mobile too. It is possible though that both values are changed at once. * if an event contains `away: false` this has the meaning of 'clear away overriding' * if an event contains `status_text: ''` this means 'clear status text' There is value in keeping our internal state simpler and consistent with what the back-end gives us as values. To achieve this we make sure if we get `away: false` and `status_text: ''` we remove the corresponding keys.

So it sounds like the answer is:

can't be undefined, nor ''

instead, will just be absent.

This is essential info for understanding the code around this type. Not only the reducer which maintains it, but also code which consumes it. And -- key test for what belongs in the code + comments, and not only commit message -- it's not at all specific to the change between this commit and its parent, and instead is 100% relevant to understanding the new code from scratch as it is, and for any future version of the codebase where these bits of code looks more or less like as they do here.

So let's stick that in the jsdoc too.

(There's also some information in there answering my questions about the semantics of the events. The same thing applies.)

gnprice · 2019-02-05T05:29:54Z

src/directSelectors.js

@@ -54,6 +55,8 @@ export const getTopics = (state: GlobalState): TopicsState => state.topics;

 export const getUserGroups = (state: GlobalState): UserGroup[] => state.userGroups;

+export const getUserStatus = (state: GlobalState): UserStatusState => state.userStatus;


gnprice · 2019-02-05T05:41:43Z

src/user-status/userStatusReducers.js

+      return {
+        ...state,
+        [action.user_id]: newUserStatus,
+      };


OK, now that I see the intended semantics for these (away has just two states, and status_text as empty-string is synonymous with missing), I think probably the cleanest thing would actually be to have a type like this in our Redux state tree: {| away: boolean, status_text: string |}. Then this code would look like

/** The status a user implicitly has when not present in the state. */ export const DEFAULT_STATUS = { away: false, status_text: '' }; // ... const status = state[action.user_id] || DEFAULT_STATUS; return { ...state, [action.user_id]: { away: action.away !== undefined ? action.away : status.away, status_text: action.status_text !== undefined ? action.status_text : status.status_text, }, };

One advantage of this would be that it makes explicit what it means when we don't have this kind of data on a given user -- unlike for many other kinds of data where it would mean "we don't know", here it means "they have these default values". The export is there for a selector to use to encapsulate that fact.

I'm OK merging a version with the existing design, though.

gnprice · 2019-02-05T05:53:49Z

OK, and just merged 5 of these commits, as ac71a73..f4283ee!

I made just one edit to their code, along with small edits to the commit messages -- fixed one straggler mention of "user status" to mean a user's presence status:

diff --git src/account-info/AccountDetails.js src/account-info/AccountDetails.js
index f06dcdfb1..66833bb24 100644
--- src/account-info/AccountDetails.js
+++ src/account-info/AccountDetails.js
@@ -17,7 +17,7 @@ const componentStyles = StyleSheet.create({
   componentListItem: {
     alignItems: 'center',
   },
-  userStatusWrapper: {
+  statusWrapper: {
     justifyContent: 'center',
     flexDirection: 'row',
   },
@@ -49,7 +49,7 @@ export default class AccountDetails extends PureComponent<Props, void> {
           shape="square"
         />
         <ComponentList outerSpacing itemStyle={componentStyles.componentListItem}>
-          <View style={componentStyles.userStatusWrapper}>
+          <View style={componentStyles.statusWrapper}>
             <PresenceStatusIndicator presence={presence} hideIfOffline={false} />
             <RawLabel style={[styles.largerText, styles.halfMarginLeft]} text={user.email} />
           </View>

Found that with git grep -iE 'user.?status'.

gnprice · 2019-02-05T05:55:56Z

(And for convenience, I just rebased the remaining 6 commits on top of those, and pushed that to the PR branch.)

borisyankov · 2019-02-05T15:35:57Z

Updated according to the latest feedback.

Since recently we are tracking all server events thus we are also getting these just-added 'user_status' events. * add a constant and an action type for this new event * start issuing a Redux action from this event

* add Flow types for a new UserStatusState * adds the base for a user status reducer that operates on a UserStatusState * adds the object to the `GlobalState` type * adds the state key to `discardKeys` and treat this data as temporary like the `presence` state * adds a `getUserStatus` direct selector

Pass `user_status` to `fetch_event_types` to receive user status data on initial load. In the reducer, handle the resulting REALM_INIT data. Also handle the usual actions that reset the state.

When a `EVENT_USER_STATUS_UPDATE` action is dispatched, handle it by updating the relevant user key or add a new key. Currently the web app does update the `away` value separately from the `status_text` string and that is the immediate plan for mobile too. It is possible though that both values are changed at once. * if an event contains `away: false` this has the meaning of 'clear away overriding' * if an event contains `status_text: ''` this means 'clear status text' There is value in keeping our internal state simpler and consistent with what the back-end gives us as values. To achieve this we make sure if we get `away: false` and `status_text: ''` we remove the corresponding keys.

gnprice · 2019-02-06T06:50:59Z

Merged -- thanks again @borisyankov !

I added under InitialDataUserStatus the comment discussed in a thread above.

I also squashed together the commit that asks the server to send that initial data, and the one that processes it in the reducer. Those feel to me like they each make more sense to understand in combination with the other, and the "request initial data" one especially is simple enough that the squashed one isn't so long as to get in the way of reading.

Based in part on these old PR comments: #3263 (comment) #3263 (comment) #2819 (review) #2983 (comment)

borisyankov force-pushed the away-status branch from 68f9ab0 to f6e95e7 Compare January 11, 2019 02:33

borisyankov changed the title ~~WIP: Away status~~ Away status Jan 11, 2019

gnprice added P1 high-priority P0 critical Highest priority and removed P1 high-priority labels Jan 14, 2019

gnprice added P1 high-priority and removed P0 critical Highest priority labels Jan 15, 2019

borisyankov added P0 critical Highest priority and removed P1 high-priority labels Jan 24, 2019

borisyankov force-pushed the away-status branch from f6e95e7 to 5ea06b2 Compare January 26, 2019 01:29

gnprice reviewed Jan 29, 2019

View reviewed changes

borisyankov force-pushed the away-status branch 11 times, most recently from 07f4d87 to 926c6ce Compare January 31, 2019 15:33

gnprice reviewed Jan 31, 2019

View reviewed changes

borisyankov force-pushed the away-status branch 6 times, most recently from c297561 to cdd37a0 Compare February 4, 2019 01:55

This was referenced Feb 4, 2019

Profile tab and UI for setting away status #3267

Closed

Display presence status taking away status into account #3326

Merged

borisyankov force-pushed the away-status branch from cdd37a0 to e383b4e Compare February 5, 2019 00:01

gnprice reviewed Feb 5, 2019

View reviewed changes

gnprice force-pushed the away-status branch from e383b4e to 58cb3a3 Compare February 5, 2019 05:54

borisyankov force-pushed the away-status branch from 58cb3a3 to 3872d15 Compare February 5, 2019 15:34

borisyankov added 4 commits February 5, 2019 22:36

user status: Recognize 'user_status' server events.

2c206a9

Since recently we are tracking all server events thus we are also getting these just-added 'user_status' events. * add a constant and an action type for this new event * start issuing a Redux action from this event

user status: Initialize data from server, and handle reset.

c8d0714

Pass `user_status` to `fetch_event_types` to receive user status data on initial load. In the reducer, handle the resulting REALM_INIT data. Also handle the usual actions that reset the state.

gnprice force-pushed the away-status branch from 3872d15 to 5d95127 Compare February 6, 2019 06:47

gnprice merged commit 5d95127 into zulip:master Feb 6, 2019

gnprice mentioned this pull request May 20, 2019

Add basic support for submessages/widgets #3220

Merged

gnprice mentioned this pull request Jun 4, 2019

Support reading and setting own Away status #3266

Closed

gnprice added a commit that referenced this pull request Dec 5, 2019

docs/style: Connecting comments, code, and commit messages.

1b6338c

Based in part on these old PR comments: #3263 (comment) #3263 (comment) #2819 (review) #2983 (comment)

gnprice added the a-presence/status Presence, and "away status" label Mar 13, 2020

		@@ -54,6 +55,8 @@ export const getTopics = (state: GlobalState): TopicsState => state.topics;

		export const getUserGroups = (state: GlobalState): UserGroup[] => state.userGroups;

		export const getUserStatus = (state: GlobalState): UserStatusState => state.userStatus;

Away status #3263

Away status #3263

Conversation

borisyankov commented Jan 10, 2019 • edited Loading

borisyankov commented Jan 11, 2019

gnprice commented Jan 14, 2019

gnprice commented Jan 14, 2019

gnprice commented Jan 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

borisyankov commented Jan 31, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnprice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnprice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnprice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnprice commented Feb 5, 2019

gnprice commented Feb 5, 2019

borisyankov commented Feb 5, 2019 • edited Loading

gnprice commented Feb 6, 2019

borisyankov commented Jan 10, 2019 •

edited

Loading

borisyankov commented Feb 5, 2019 •

edited

Loading