On message touch, do a narrow #333

borisyankov · 2017-02-14T16:46:01Z

Implements #306

smarx · 2017-02-14T16:46:04Z

Automated message from Dropbox CLA bot

@borisyankov, it looks like you've already signed the Dropbox CLA. Thanks!

neerajwahi

I get exceptions when I run this in the simulator and click on a message:

neerajwahi · 2017-02-15T07:30:50Z

src/message-list/renderMessages.js

        key={item.id}
        auth={auth}
-        messageId={item.id}
+        message={item}


I like this simplification!

borisyankov · 2017-02-15T18:03:55Z

Can not reproduce.
Also, likely not connected to this PR.

neerajwahi · 2017-02-15T18:11:19Z

You're right. I must have gotten into some weird state :)

I think this diff needs a bit of a manual rebase against master because there have been some changes that git won't catch. For example, doNarrow takes a second parameter with the message to show initially when narrowing (so that clicking on a message header doesn't jump to the latest message in the narrow).

neerajwahi · 2017-02-15T18:13:17Z

src/message-list/renderMessages.js

@@ -59,16 +59,11 @@ export default ({ auth, subscriptions, messages, narrow, mute, doNarrow }) => {
    list.push(
      <MessageContainer
        type="message"
-        id={item.id}


We also need to change MessageList to look at props.message.id instead of props.id when generating its list of 'anchor' messages (otherwise scrolling gets messed up)

Implements zulip#306

borisyankov · 2017-02-15T22:12:21Z

Fixed the email issue too.

There's no reason we would sometimes want a stream narrow here. This is just how it was when this function was first written, for narrowing to a message's conversation, in 34ab1f9 (zulip#333) in 2017. This case also should be impossible anyway, because message topics should always be nonempty -- we convert empty to "(no topic)".

There's no reason we would sometimes want a stream narrow here. This is just how it was when this function was first written (as `getNarrowFromMessage`), for narrowing to a message's conversation, in 34ab1f9 (zulip#333) in 2017. This case also should be impossible anyway, because message topics should always be nonempty -- we convert empty to "(no topic)". Co-authored-by: Chris Bobbe <cbobbe@zulip.com>

There's no reason we would sometimes want a stream narrow here. This is just how it was when this function was first written, for narrowing to a message's conversation, in 34ab1f9 (zulip#333) in 2017. This case also should be impossible anyway, because message topics should always be nonempty -- we convert empty to "(no topic)".

There's no reason we would sometimes want a stream narrow here. This is just how it was when this function was first written (as `getNarrowFromMessage`), for narrowing to a message's conversation, in 34ab1f9 (zulip#333) in 2017. This case also should be impossible anyway, because message topics should always be nonempty -- we convert empty to "(no topic)". Co-authored-by: Chris Bobbe <cbobbe@zulip.com>

There's no reason we would sometimes want a stream narrow here. This is just how it was when this function was first written, for narrowing to a message's conversation, in 34ab1f9 (zulip#333) in 2017. This case also should be impossible anyway, because message topics should always be nonempty -- we convert empty to "(no topic)".

There's no reason we would sometimes want a stream narrow here. This is just how it was when this function was first written (as `getNarrowFromMessage`), for narrowing to a message's conversation, in 34ab1f9 (zulip#333) in 2017. This case also should be impossible anyway, because message topics should always be nonempty -- we convert empty to "(no topic)". Co-authored-by: Chris Bobbe <cbobbe@zulip.com>

neerajwahi suggested changes Feb 15, 2017

View reviewed changes

src/message-list/renderMessages.js

key={item.id}

auth={auth}

messageId={item.id}

message={item}

Copy link

Member

neerajwahi Feb 15, 2017

Choose a reason for hiding this comment

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

I like this simplification!

neerajwahi reviewed Feb 15, 2017

View reviewed changes

borisyankov force-pushed the fix306 branch from 5813243 to 3cc54fb Compare February 15, 2017 22:04

On message touch, do a narrow

2483b48

Implements zulip#306

borisyankov force-pushed the fix306 branch from 3cc54fb to 2483b48 Compare February 15, 2017 22:11

borisyankov merged commit 34ab1f9 into zulip:master Feb 15, 2017

borisyankov deleted the fix306 branch February 21, 2017 20:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

On message touch, do a narrow #333

On message touch, do a narrow #333

borisyankov commented Feb 14, 2017

smarx commented Feb 14, 2017

neerajwahi left a comment •

edited

Loading

neerajwahi Feb 15, 2017

borisyankov commented Feb 15, 2017

neerajwahi commented Feb 15, 2017

neerajwahi Feb 15, 2017

borisyankov Feb 15, 2017

borisyankov commented Feb 15, 2017

On message touch, do a narrow #333

On message touch, do a narrow #333

Conversation

borisyankov commented Feb 14, 2017

smarx commented Feb 14, 2017

neerajwahi left a comment • edited Loading

Choose a reason for hiding this comment

neerajwahi Feb 15, 2017

Choose a reason for hiding this comment

borisyankov commented Feb 15, 2017

neerajwahi commented Feb 15, 2017

neerajwahi Feb 15, 2017

Choose a reason for hiding this comment

borisyankov Feb 15, 2017

Choose a reason for hiding this comment

borisyankov commented Feb 15, 2017

neerajwahi left a comment •

edited

Loading