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

Scratch Comments Feature #1230

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from
Draft

Conversation

conorgolden1
Copy link
Contributor

@conorgolden1 conorgolden1 commented May 9, 2024

Feedback is welcome

Features

  • Model
  • Pagination
  • Finished views
  • Able to post comment
  • Refresh Link on posted comment || Live Feed
  • Stylized Comments
  • Editable Comments
  • Add DateTime for Comments
  • Ability to delete own comments
  • Profile pictures with comments
  • User moderation of comments on scratch
  • Admin moderation capabilities
  • Comments listed on user profile
  • Finished Scratch Comment Panel
  • Linkable comments
  • Replace TimeAgo with date-fns
  • Mobile view

Tests

Comment

  • Creation
  • Deletion
  • Update

Most recent example

Screencast.from.2024-05-16.00-03-08.webm

fixes #128

Copy link
Member

@bates64 bates64 left a comment

Choose a reason for hiding this comment

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

I'm liking this so far, just some comments

@@ -0,0 +1,78 @@
.holder {
Copy link
Member

Choose a reason for hiding this comment

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

New code should use Tailwind where possible rather than SCSS modules

overflow: hidden;
resize: none;

&::-webkit-input-placeholder {
Copy link
Member

Choose a reason for hiding this comment

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

}
}

function EditComment({ comment, stopEditing, submit }: { comment: any, stopEditing: () => void, submit: (text: string) => Promise<unknown> }) {
Copy link
Member

Choose a reason for hiding this comment

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

unknown!?

Also split this signature onto multiple lines

Comment on lines +27 to +30
await api.delete_(commentUrl(comment), {})
alert("Comment deleted")
} catch (e) {
alert(`Error Deleting Comment: ${e}`)
Copy link
Member

Choose a reason for hiding this comment

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

TODO this should be a dialog or a toast or something

<div className={styles.counter}>{text.length} / {maxTextLength}</div>
</div>
</section>
<br />
Copy link
Member

Choose a reason for hiding this comment

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

Why?

}

return (
<div>
Copy link
Member

Choose a reason for hiding this comment

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

Ideally this would be more semantic, like a form, and editing would submit the form

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

Successfully merging this pull request may close these issues.

Scratch comments
2 participants