-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add ability to delete and re-draft a comment with no replies + markdown infos + unicode emojis #3046
Add ability to delete and re-draft a comment with no replies + markdown infos + unicode emojis #3046
Conversation
client/src/app/+videos/+video-watch/comment/video-comment.component.html
Show resolved
Hide resolved
Nice! I like the markdown icon but at the same time I'm not too sure about it's position. Usually this is right aligned in the input, and greyed like the input border. |
fe02077
to
e4c91ba
Compare
@rigelk, was it what you thought ? |
The last commit adds unicode emojis to markdown (depending on Operating System it will have a different display https://apps.timwhitlock.info/emoji/tables/unicode) |
@kimsible that is what I though, yes :) Could you please fix https://github.com/Chocobozzz/PeerTube/pull/3046/checks?check_run_id=961331548#step:6:126? |
Thanks, I'm working on it. For the emojis, I don't know if we must include an emoji collection into PeerTube, in the best way, yes, because some people do not have every one on their operating system (especially on desktop linux and windows). What do we do ? |
I'm inclined to look at what Mastodon does, where they adopted a full emoji management interface that is highly extensible. Until then, only including a light set like It's not like PeerTube is only focusing on text and thus very reliant on emojis, but emojis have their place too in the comment palette (i.e. an issue has already considered video reactions - a much heavier development - which proves there is interest for richer expression forms to react to videos). |
Yes, the light version sounds reasonable thanks.
I think you meant this PR #3026 ? |
@kimsible both of them actually, though they are just issues, not PRs. |
Ok @rigelk , so you mean adding a Popover with emojis ? Otherwise, I'm not sure (since I'm not an expert of Typescript) but light version emoji of markdown-it seems to not be implemented to |
no, I didn't mean anything like this. The issue I quoted was merely an example of how text is arguably not the only response type we should consider, as an argument in favor of including emojis (since you asked if we should include emojis). Back on your current error, leave it at the typical |
3652611
to
02b3ac6
Compare
client/src/app/+videos/+video-watch/comment/video-comment-add.component.ts
Outdated
Show resolved
Hide resolved
client/src/app/+videos/+video-watch/comment/video-comment-add.component.ts
Outdated
Show resolved
Hide resolved
client/src/app/+videos/+video-watch/comment/video-comment.component.html
Outdated
Show resolved
Hide resolved
client/src/app/+videos/+video-watch/comment/video-comment.component.ts
Outdated
Show resolved
Hide resolved
client/src/app/+videos/+video-watch/comment/video-comment.component.ts
Outdated
Show resolved
Hide resolved
client/src/app/+videos/+video-watch/comment/video-comments.component.ts
Outdated
Show resolved
Hide resolved
client/src/app/+videos/+video-watch/comment/video-comments.component.ts
Outdated
Show resolved
Hide resolved
d5c6cfe
to
a101280
Compare
Thanks for the review @Chocobozzz, I've fixed / refactored all your comments. |
@Chocobozzz maybe this PR should have been merged before changing localization system, we need to re-pass on all files. Except this.i18n('') to $localize``, templates with i18n attributes must be changed too ? |
@kimsible templates are not touched, just their associated components logic as the I18n service model is replaced. |
Yes, since templates are not affected. |
I'll merge and fix conflicts of this PR myself. |
d3464a2
to
2a7eab0
Compare
Thanks @Chocobozzz, I've already merged it but maybe you would like to check if I missed something. |
2a7eab0
to
cd28662
Compare
Reply
value of add comment button toComment
only for comment main thread textarea ;