Replies #1741

t3chguy · 2018-02-10T11:21:09Z

Signed-off-by: Michael Telatynski [email protected]

Requires matrix-org/matrix-js-sdk#607

Requires element-hq/element-web#6117

Fixes:

Inline permalinks get converted to quotes element-hq/element-web#6065
Replying to a message with an emote doesn't look like an emote element-hq/element-web#6061
Replying to events can crash react element-hq/element-web#6059
Rich Quoting conflicts with Search Results element-hq/element-web#5982
Plaintext representation of nested rich replies get mangled at some point element-hq/element-web#5974
Nested rich replies are an endless ping fest. element-hq/element-web#5968
Instead of "Quote" for the text in a rich reply, say something like "In reply to..."? element-hq/element-web#5967
Quote and QuotePreview do not specify onWidgetLoad on EventTile element-hq/element-web#6168

Signed-off-by: Michael Telatynski <[email protected]>

…rix-org/matrix-react-sdk into t3chguy/m.relates_to

t3chguy · 2018-02-10T11:28:05Z

Seems like this is being built against the wrong js-sdk version
Riot-web tests (which include the react-sdk tests) pass, as they use the correct branches all the way soooo

Signed-off-by: Michael Telatynski <[email protected]>

t3chguy · 2018-02-10T12:43:51Z

~~Introduces one issue which I would like to fix before merging, on-hover of the Reply-set, the e2e icons and timestamp shuffle to the left, VERY ANNOYINGLY~~

FIXED

Signed-off-by: Michael Telatynski <[email protected]>

t3chguy · 2018-02-10T16:01:25Z

tulir · 2018-02-11T11:36:57Z

When I send an event that has an empty object as m.relates_to, Riot UI starts lagging a lot and the event doesn't appear. The console shows this when I try to switch to a room that has such an event (and switching fails):

Signed-off-by: Michael Telatynski <[email protected]>

…rix-org/matrix-react-sdk into t3chguy/m.relates_to

lukebarnard1 · 2018-02-16T15:29:29Z

OK, so I've had a play with this, and seems really really cool 🙂 VERY cool to see permalinks that can link to entire discussions by just linking the last event in the thread (ignoring branching potential)

I found a few bugs:

The timeline doesn't scroll down entirely to the bottom after sending a reply (sometimes).
The timeline doesn't scroll down entirely to the bottom when expanding replies (sometimes - seemingly only when expanding to view not the oldest message).
When switching to a room with replies, the timeline doesn't scroll down to the bottom if it was scrolled down to the bottom before switching to it.

Also, I had some design-related thoughts. If there's already been a design decision then fair enough. If @lampholder or @ara4n have thoughts on my thoughts, that'd be useful:

You don't get to see a preview of the entire previous thread when replying.
Timestamps for entire thread are shown if the thread is attached to the most recent message. Showing all timestamps for a thread on hover is a bit weird, but I suppose that's ok for now.
Clicking "in reply to" is a bit unintuitive, especially as it looks like someone has typed that out. I would prefer some sort of "more..." indicator.
As much as I love the use of Pills in general, it wasn't inserted by someone into the actual message so I think I'd prefer the non-Pill equivalent. Also whatever it is, clicking on it should show MemberInfo.

lukebarnard1 · 2018-02-16T15:30:37Z

FTR the tests fail because js-sdk develop is being used for the tests as opposed to the one we intend to merge at the same time as this. Ideally we'd merge the js-sdk branch first and then restart the tests here.

t3chguy · 2018-02-16T15:37:55Z

You don't get to see a preview of the entire previous thread when replying.

is a thing I broke but will re-add

t3chguy · 2018-02-16T15:39:28Z

Re

Clicking "in reply to" is a bit unintuitive, especially as it looks like someone has typed that out. I would prefer some sort of "more..." indicator.

and

As much as I love the use of Pills in general, it wasn't inserted by someone into the actual message so I think I'd prefer the non-Pill equivalent. Also whatever it is, clicking on it should show MemberInfo.

Design input welcome

t3chguy · 2018-02-16T15:40:57Z

(ignoring branching potential)

thats the whole boat of replies vs swimlanes (threading)
the latter of which would be really inefficient to resolve without server indexed aid

Signed-off-by: Michael Telatynski <[email protected]>

lukebarnard1

I suggest running tests locally and making sure they pass.

lukebarnard1 · 2018-05-01T12:00:14Z

src/components/views/rooms/MessageComposerInput.js


-        sendMessagePromise.done((res) => {
+        this.client.sendMessage(this.props.room.roomId, content).done((res) => {


Running tests locally, I found that this.client might not be an object at this point.

t3chguy · 2018-05-01T12:15:16Z

this.client might not be an object at this point.

It will always be an object as the constructor sets it as such, just that sinon overwrites it with a stub that doesn't have sendMessage

lukebarnard1 · 2018-05-01T12:22:13Z

Ah ok, looks like a simple fix of adding sendMessage to the stub then.

…ssage Signed-off-by: Michael Telatynski <[email protected]>

t3chguy · 2018-05-01T12:27:09Z

Ah ok, looks like a simple fix of adding sendMessage to the stub then.

not quite, all the tests have to be refactored due to relying on checking whether the sendHtmlMessage and sendTextMessage get called but this is done now

lukebarnard1 · 2018-05-01T16:01:49Z

The tests pass! (locally) 🎉

lukebarnard1

Almost there!

lukebarnard1 · 2018-05-01T16:08:34Z

src/components/structures/RoomView.js

@@ -916,9 +916,14 @@ module.exports = React.createClass({

        ContentMessages.sendContentToRoom(
            file, this.state.room.roomId, MatrixClientPeg.get(),
-        ).catch((error) => {
+        ).done(() => {


"any unhandled rejection that ends up here will ... be thrown as an error" (see http://bluebirdjs.com/docs/api/done.html)

this should be catch(...).then(...)

lukebarnard1 · 2018-05-01T16:16:24Z

src/components/views/elements/ReplyThread.js

+        }
+    }
+
+    static async getEvent(room, eventId) {


I really think these statics should be defined after the constructor. That way, you can read down from componentWillMount to initialize instead of scrolling up through the statics.

lukebarnard1 · 2018-05-01T16:34:17Z

src/components/views/rooms/ReplyPreview.js

            </div>
        </div>;
    }
 }
+
+function dummyOnWidgetLoad() {}


Maybe leave a comment explaining that onWidgetLoad should be made optional in EventTile

Signed-off-by: Michael Telatynski <[email protected]>

lukebarnard1 · 2018-05-02T09:21:58Z

src/components/structures/RoomView.js

+            dis.dispatch({
+                action: 'message_sent',
+            });
+        }).catch((error) => {


This error could have been thrown in the then. Ultimately we want two branches. One that is called if the call was successful, one otherwise.

In a sync world, you'd do:

try { thingThatMightFail(); } catch (err) { handle(err); return; } success();

or equally with async await. Perhaps just make the function async.

Sure, have done; but cannot do the same for handleReturn in MCI as it requires the boolean return type for Draft-js

lukebarnard1 · 2018-05-02T10:43:34Z

Also. #1869 has landed so the tests will pass if develop is merged into this branch. (Thanks @dbkr!)

…rix-org/matrix-react-sdk into t3chguy/m.relates_to

Signed-off-by: Michael Telatynski <[email protected]>

initial refactor of Replies to use B explicit over-the-wire format

1c3d8cb

Signed-off-by: Michael Telatynski <[email protected]>

t3chguy mentioned this pull request Feb 10, 2018

Replies styling element-hq/element-web#6117

Closed

Merge branches 'develop' and 't3chguy/m.relates_to' of github.com:mat…

a15d024

…rix-org/matrix-react-sdk into t3chguy/m.relates_to

t3chguy mentioned this pull request Feb 10, 2018

Replies matrix-org/matrix-js-sdk#607

Merged

t3chguy added 3 commits February 10, 2018 12:31

fix Replies for e2e

1814546

Signed-off-by: Michael Telatynski <[email protected]>

mass s/Quote/Reply

fbb950e

Signed-off-by: Michael Telatynski <[email protected]>

pass onWidgetLoad to fix console err for e2e

23c0daa

Signed-off-by: Michael Telatynski <[email protected]>

t3chguy added 3 commits February 10, 2018 15:45

tidy, improve error handling and improve Replies, run gen-i18n

7425d01

Signed-off-by: Michael Telatynski <[email protected]>

make spinner size more appropriate

25bc9cf

Signed-off-by: Michael Telatynski <[email protected]>

add unmounted guard

7bf05b0

Signed-off-by: Michael Telatynski <[email protected]>

This was referenced Feb 10, 2018

Update native reply format mautrix/telegram#57

Closed

Update native reply format mautrix/telegram#58

Merged

fix weird in op behaviour with null

aeedf48

Signed-off-by: Michael Telatynski <[email protected]>

This was referenced Feb 13, 2018

Change Rich Quoting HTML Format over the wire #1701

Closed

Quote and QuotePreview do not specify onWidgetLoad on EventTile element-hq/element-web#6168

Closed

lukebarnard1 self-assigned this Feb 16, 2018

Merge branches 'develop' and 't3chguy/m.relates_to' of github.com:mat…

2e3cbb3

…rix-org/matrix-react-sdk into t3chguy/m.relates_to

t3chguy added 4 commits February 16, 2018 20:58

show replies in ReplyPreview

8062494

Signed-off-by: Michael Telatynski <[email protected]>

reply and reply_preview only differ in whether they contain <Reply />

cb293a8

Signed-off-by: Michael Telatynski <[email protected]>

fix 12/24h in Reply/ReplyPreview

f1a3592

Signed-off-by: Michael Telatynski <[email protected]>

fix Composer focus after twiddling with Replies

0d6fc9b

Signed-off-by: Michael Telatynski <[email protected]>

t3chguy assigned lukebarnard1 and unassigned t3chguy Apr 30, 2018

lukebarnard1 suggested changes May 1, 2018

View reviewed changes

lukebarnard1 assigned t3chguy and unassigned lukebarnard1 May 1, 2018

Refactor tests to understand sendMessage instead of send{Text,Html}Me…

f0bd4a5

…ssage Signed-off-by: Michael Telatynski <[email protected]>

t3chguy assigned lukebarnard1 and unassigned t3chguy May 1, 2018

lukebarnard1 suggested changes May 1, 2018

View reviewed changes

lukebarnard1 assigned t3chguy and unassigned lukebarnard1 May 1, 2018

t3chguy added 3 commits May 1, 2018 17:42

re-order ReplyThread methods for readability

11ae080

Signed-off-by: Michael Telatynski <[email protected]>

avoid .done and .then anti-pattern

db55f87

Signed-off-by: Michael Telatynski <[email protected]>

clean up EventTile:onWidgetLoad optional/required mess

ec4ec47

Signed-off-by: Michael Telatynski <[email protected]>

t3chguy assigned lukebarnard1 and unassigned t3chguy May 1, 2018

lukebarnard1 suggested changes May 2, 2018

View reviewed changes

lukebarnard1 assigned t3chguy and unassigned lukebarnard1 May 2, 2018

t3chguy added 2 commits May 2, 2018 13:08

Merge branches 'develop' and 't3chguy/m.relates_to' of github.com:mat…

f2102e2

…rix-org/matrix-react-sdk into t3chguy/m.relates_to

switch RoomView:uploadFile to async to clean up then/catch handling

4c3f811

Signed-off-by: Michael Telatynski <[email protected]>

t3chguy assigned lukebarnard1 and unassigned t3chguy May 2, 2018

lukebarnard1 approved these changes May 2, 2018

View reviewed changes

lukebarnard1 merged commit 6bf1eb1 into develop May 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replies #1741

Replies #1741

t3chguy commented Feb 10, 2018 •

edited

Loading

t3chguy commented Feb 10, 2018 •

edited

Loading

t3chguy commented Feb 10, 2018 •

edited

Loading

t3chguy commented Feb 10, 2018

tulir commented Feb 11, 2018

lukebarnard1 commented Feb 16, 2018 •

edited

Loading

lukebarnard1 commented Feb 16, 2018

t3chguy commented Feb 16, 2018

t3chguy commented Feb 16, 2018

t3chguy commented Feb 16, 2018

lukebarnard1 left a comment

lukebarnard1 May 1, 2018

t3chguy commented May 1, 2018

lukebarnard1 commented May 1, 2018

t3chguy commented May 1, 2018

lukebarnard1 commented May 1, 2018

lukebarnard1 left a comment

lukebarnard1 May 1, 2018

lukebarnard1 May 1, 2018

lukebarnard1 May 1, 2018

lukebarnard1 May 2, 2018

t3chguy May 2, 2018

lukebarnard1 commented May 2, 2018


		sendMessagePromise.done((res) => {
		this.client.sendMessage(this.props.room.roomId, content).done((res) => {

Replies #1741

Replies #1741

Conversation

t3chguy commented Feb 10, 2018 • edited Loading

Requires matrix-org/matrix-js-sdk#607

Requires element-hq/element-web#6117

t3chguy commented Feb 10, 2018 • edited Loading

t3chguy commented Feb 10, 2018 • edited Loading

t3chguy commented Feb 10, 2018

tulir commented Feb 11, 2018

lukebarnard1 commented Feb 16, 2018 • edited Loading

lukebarnard1 commented Feb 16, 2018

t3chguy commented Feb 16, 2018

t3chguy commented Feb 16, 2018

t3chguy commented Feb 16, 2018

lukebarnard1 left a comment

Choose a reason for hiding this comment

lukebarnard1 May 1, 2018

Choose a reason for hiding this comment

t3chguy commented May 1, 2018

lukebarnard1 commented May 1, 2018

t3chguy commented May 1, 2018

lukebarnard1 commented May 1, 2018

lukebarnard1 left a comment

Choose a reason for hiding this comment

lukebarnard1 May 1, 2018

Choose a reason for hiding this comment

lukebarnard1 May 1, 2018

Choose a reason for hiding this comment

lukebarnard1 May 1, 2018

Choose a reason for hiding this comment

lukebarnard1 May 2, 2018

Choose a reason for hiding this comment

t3chguy May 2, 2018

Choose a reason for hiding this comment

lukebarnard1 commented May 2, 2018

t3chguy commented Feb 10, 2018 •

edited

Loading

t3chguy commented Feb 10, 2018 •

edited

Loading

t3chguy commented Feb 10, 2018 •

edited

Loading

lukebarnard1 commented Feb 16, 2018 •

edited

Loading