Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Conversation

@bwindels
Copy link
Contributor

@bwindels bwindels commented Aug 23, 2019

Implements element-hq/element-web#10629

To support auto-replacing emoticons, which can span multiple parts, the model gains support for a transform step where a range can be opened on the model, and replace with other parts. This is used to find and replace emoticons.

emoticons

@bwindels bwindels force-pushed the bwindels/cider-replace-emoticons branch from aa2fe1e to 0273795 Compare August 26, 2019 14:17
@bwindels bwindels marked this pull request as ready for review August 26, 2019 14:21
@bwindels bwindels requested a review from a team August 26, 2019 14:23
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

The tests really help with understanding this code, so thank you for including them :)

I have documentation concerns, but nothing too controversial I hope.

import {renderModel} from '../../../editor/render';
import {Room} from 'matrix-js-sdk';
import TypingStore from "../../../stores/TypingStore";
import EMOJIBASE from 'emojibase-data/en/compact.json';
Copy link
Member

Choose a reason for hiding this comment

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

we should consider importing the user's language in the future, if that's even a thing that makes sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure, are emoticons internationalized? That's the only thing we use it for here, mapping emoticons to emojis. Textual description is not used here.

Copy link
Member

Choose a reason for hiding this comment

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

ah, if we're just using their shapes and not descriptions it is probably fine. There's probably some locale-specific emoji out there, but I don't think we've ever received a request for them.

as the amount of characters might not have changed,
parts may still have been merged, removed or added which
requires a new position.
also add more inline comments to explain what is going on
@bwindels
Copy link
Contributor Author

bwindels commented Aug 27, 2019

Found another bug where at the start of the editor typing ";) " would replace with ";😉 ", fixed now.

@bwindels bwindels requested a review from turt2live August 27, 2019 08:56
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

still magic to me

@bwindels bwindels merged commit e0b99b5 into develop Aug 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants