Skip to content

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Feb 7, 2020

This clears up a swath of the confusion around the concept of "anchor" that I dug into following #3877 .

@gnprice gnprice requested a review from chrisbobbe February 7, 2020 05:27
This isn't really contributing anything useful to add to the layers
immediately above and below it.

The name is pretty mysterious about what it means, too.  We'll
clean up some other names around here, but easiest to have one
fewer to deal with.
This is rather more explicit about what this selector actually means.

To the extent that "anchor" conveys any meaning of its own, it's an
answer to a UI question of how the message list should behave.  And
that's a question whose answer should live in the MessageList UI code,
not in a selectors module.
Zero is never a great choice for a "no such item" sentinel where
a `number` is expected -- it makes it likely the sentinel will be
implicitly treated like just another number.

Here, it turns out that where the value ultimately gets consumed
(in `scrollToAnchor`), it has the effect of causing no message
node to be found... so that we scroll to the *bottom*.  Make that
path through the control flow explicit.

In this case, 0 is an especially confusing value to use for "there
is no unread message" because this value is called "anchor"... and
in other places called "anchor", we use 0 (as FIRST_UNREAD_ANCHOR)
to mean the first unread message!
This makes it a lot more explicit what this means -- what an
"anchor" has to do with the display of the message list.
@chrisbobbe chrisbobbe merged commit d74696b into zulip:master Feb 11, 2020
@gnprice gnprice deleted the pr-msglist-anchor branch February 11, 2020 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants