typing_status: Switch sentinel "recipient" value to `null`.

This feels a bit more semantically appropriate: it more clearly says
"here's some information: there is no (relevant) recipient", rather
than "no information available".  (Both `null` and `undefined` in JS
can have either meaning, but `undefined` especially commonly means
the latter.)

Concretely, it ensures a bit more explicitness where the value
originates: a bare `return;` becomes `return null;`, reflecting the
fact that it is returning a quite informative value.

Also make the implementation more explicit about what's expected here,
replacing truthiness tests with `!== null`.  (A bit more idiomatic
would be `!= null`, which is equivalent when the value is well-typed
and a bit more robust to ill-typing bugs.  But lint complains about
that version.)
This commit is contained in:
Greg Price 2019-10-21 17:52:01 -07:00
parent a191890213
commit 71596648c2
4 changed files with 17 additions and 17 deletions

View File

@ -16,7 +16,7 @@ run_test('basics', () => {
// invalid conversation basically does nothing
var worker = {};
typing_status.update(worker, undefined);
typing_status.update(worker, null);
// Start setting up more testing state.
typing_status.initialize_state();
@ -128,7 +128,7 @@ run_test('basics', () => {
});
// Call stop with nothing going on.
call_handler(undefined);
call_handler(null);
assert.deepEqual(typing_status.state, {
next_send_start_time: undefined,
idle_timer: undefined,
@ -158,7 +158,7 @@ run_test('basics', () => {
assert(events.idle_callback);
// Explicitly stop alice.
call_handler(undefined);
call_handler(null);
assert.deepEqual(typing_status.state, {
next_send_start_time: undefined,
idle_timer: undefined,
@ -188,7 +188,7 @@ run_test('basics', () => {
assert(events.idle_callback);
// Switch to an invalid conversation.
call_handler(undefined);
call_handler(null);
assert.deepEqual(typing_status.state, {
next_send_start_time: undefined,
idle_timer: undefined,
@ -202,7 +202,7 @@ run_test('basics', () => {
});
// Switch to another invalid conversation.
call_handler(undefined);
call_handler(null);
assert.deepEqual(typing_status.state, {
next_send_start_time: undefined,
idle_timer: undefined,

View File

@ -26,7 +26,7 @@ function send_typing_notification_ajax(user_ids_array, operation) {
function get_user_ids_array() {
var user_ids_string = compose_pm_pill.get_user_ids_string();
if (user_ids_string === "") {
return;
return null;
}
return people.user_ids_string_to_ids_array(user_ids_string);
@ -65,14 +65,14 @@ exports.initialize = function () {
// If our previous state was no typing notification, send a
// start-typing notice immediately.
var new_recipient =
is_valid_conversation() ? exports.get_recipient() : undefined;
is_valid_conversation() ? exports.get_recipient() : null;
typing_status.update(worker, new_recipient);
});
// We send a stop-typing notification immediately when compose is
// closed/cancelled
$(document).on('compose_canceled.zulip compose_finished.zulip', function () {
typing_status.update(worker, undefined);
typing_status.update(worker, null);
});
};

View File

@ -17,8 +17,8 @@ export const state = {};
/** Exported only for tests. */
export function initialize_state() {
state.current_recipient = undefined;
state.next_send_start_time = undefined;
state.current_recipient = null;
state.next_send_start_time = undefined;
state.idle_timer = undefined;
}
@ -81,9 +81,9 @@ export function maybe_ping_server(worker, recipient) {
* composing a stream message should be treated like composing no message at
* all.
*
* Call with `new_recipient` of `undefined` when the user actively stops
* Call with `new_recipient` of `null` when the user actively stops
* composing a message. If the user switches from one set of recipients to
* another, there's no need to call with `undefined` in between; the
* another, there's no need to call with `null` in between; the
* implementation tracks the change and behaves appropriately.
*
* See docs/subsystems/typing-indicators.md for detailed background on the
@ -92,12 +92,12 @@ export function maybe_ping_server(worker, recipient) {
* @param {*} worker Callbacks for reaching the real world. See typing.js
* for implementations.
* @param {*} new_recipient The users the PM being composed is addressed to,
* as a sorted array of user IDs; or `undefined` if no PM is being
* composed anymore.
* as a sorted array of user IDs; or `null` if no PM is being composed
* anymore.
*/
export function update(worker, new_recipient) {
var current_recipient = state.current_recipient;
if (current_recipient) {
if (current_recipient !== null) {
// We need to use _.isEqual for comparisons; === doesn't work
// on arrays.
if (_.isEqual(new_recipient, current_recipient)) {
@ -117,7 +117,7 @@ export function update(worker, new_recipient) {
stop_last_notification(worker);
}
if (!new_recipient) {
if (new_recipient === null) {
// If we are not talking to somebody we care about,
// then there is no more action to take.
return;

View File

@ -14,5 +14,5 @@ type Worker = {|
declare export function update(
worker: Worker,
new_recipient: RecipientUserIds | void,
new_recipient: RecipientUserIds | null
): void;