-
-
Notifications
You must be signed in to change notification settings - Fork 831
Basic diff visualisation for plain text edits #3238
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks quite cool! 😁
@@ -115,16 +126,36 @@ export default class EditHistoryMessage extends React.PureComponent { | |||
); | |||
} | |||
|
|||
_diffIt(oldBody, newBody) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Maybe just _diff
? Unsure why this method would use an It
suffix...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, forgot to rename after prototype code :)
function isPlainMessage(event) { | ||
const content = getReplacedContent(event); | ||
return content.msgtype === "m.text" && !content.format; | ||
} | ||
|
||
export default class EditHistoryMessage extends React.PureComponent { | ||
static propTypes = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add missing prop types.
@@ -66,6 +66,7 @@ | |||
"classnames": "^2.1.2", | |||
"commonmark": "^0.28.1", | |||
"counterpart": "^0.18.0", | |||
"diff-match-patch": "^1.0.4", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a big dependency by any chance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's 75kb of JS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I assume that's not too much in the scheme of things...
Part of element-hq/element-web#9493