Skip to content
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

Comments functionality #1166

Closed
wants to merge 24 commits into from
Closed

Conversation

andrewmurraydavid
Copy link
Member

@andrewmurraydavid andrewmurraydavid commented Oct 3, 2022

Created the comments feature to enable the use of Rich text using the EditorJs library.

Currently the only Commentable we have is the Project object and we're possibly looking into making other object commentable as well. Because this PR is not as much focused on the Comments feature but more on the Rich text, we put aside some API work which was not critical to the Rich text feature.

┆Issue is synchronized with this Monday item by Unito
┆Link To Item: https://seed-company-squad.monday.com/boards/3451697530/pulses/3466486753

@andrewmurraydavid andrewmurraydavid linked an issue Oct 3, 2022 that may be closed by this pull request
@andrewmurraydavid andrewmurraydavid self-assigned this Oct 18, 2022
@andrewmurraydavid andrewmurraydavid marked this pull request as ready for review October 18, 2022 12:56
@andrewmurraydavid andrewmurraydavid changed the title WIP Comments functionality Comments functionality Oct 18, 2022
Copy link
Member

@CarsonF CarsonF left a comment

Choose a reason for hiding this comment

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

Progress!

UI Comments

Screen Shot 2022-10-18 at 9 26 02 AM
The title/close button need some work. The icon is stretched instead of giving it its proper space.
The title style seems a little weird too... maybe an h3 would be better.


Screen Shot 2022-10-18 at 9 29 02 AM
I think the title could be dropped here and the placeholder changed to "Write a comment..."


Screen Shot 2022-10-18 at 9 39 47 AM
These shadows look weird. Each thread should be at the same "0" elevation in the list, so just like a line border appearance. It was just the thread replies that were supposed to look like a "sub-drawer" that was "further below" the list of threads.


Screen Shot 2022-10-18 at 9 44 10 AM
This doesn't seem the best. Perhaps we can just have a reply button in the bottom right of the comment thread instead?


Screen Shot 2022-10-18 at 9 48 15 AM

  • "Hide Replies" doesn't look that good. What if we did an expand icon in the bottom right, below the context button?
    Screen Shot 2022-10-18 at 9 51 27 AM
    Maybe a "Reply" button could sit to the left of that?
  • I'm not happy with the indentation of the replies. I think that's valuable real estate to be consuming. I think if we can make the "sub drawer" appear "under" more that would be better. It's also two competing designs trying to communicate the same thing: shadows and indentation. If the shadows are not enough, maybe a different subtle background shade could help?

src/TestContext.tsx Outdated Show resolved Hide resolved
src/common/urls.ts Outdated Show resolved Hide resolved
src/components/EditorJsWrapper/editorJsTools.ts Outdated Show resolved Hide resolved
src/components/EditorJsWrapper/editorJsTools.ts Outdated Show resolved Hide resolved
src/scenes/Root/CommentsBar/CommentItem/CommentItem.tsx Outdated Show resolved Hide resolved
src/scenes/Root/CommentsBar/CommentReply/CommentReply.tsx Outdated Show resolved Hide resolved
@andrewmurraydavid
Copy link
Member Author

@CarsonF , we should probably just close this as it has been stale for a while and brings no value.

@sethmcknight
Copy link
Member

This lived to the end of its lifetime and died

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RichText via Editor.js
3 participants