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

fix: add a close button to comment input #3897

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tu2463
Copy link

@tu2463 tu2463 commented Dec 2, 2024

Changes

This PR resolves #1606 dailydotdev/daily#1606

  • Added a CloseButton component to the MarkdownInput component, so that we can see a "X" on the top-right corner of the comment box.
image
  • Let CommentInputOrModal passes onClose to CommentMarkdownInput as a prop "onRequestClose". This function is eventually passed to MarkdownInput, so that clicking the close button can cancel the comment.

Manual Testing

Caution

Please make sure existing components are not breaking/affected by this PR

  1. Go to a post
  2. Click on comment button
  3. Should see the comment box with a close button. Clicking the close button will cancel the comment.

-->

Copy link

vercel bot commented Dec 2, 2024

@tu2463 is attempting to deploy a commit to the Daily Dev Team on Vercel.

A member of the Team first needs to authorize it.

Comment on lines +187 to +191
<CloseButton
size={ButtonSize.XSmall}
className="absolute right-1 top-1 laptop:right-3 laptop:top-3"
onClick={onRequestClose}
/>
Copy link
Member

Choose a reason for hiding this comment

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

There can be places where this was used, to support backwards compat, and only show this when needed, let's do a conditional.

Suggested change
<CloseButton
size={ButtonSize.XSmall}
className="absolute right-1 top-1 laptop:right-3 laptop:top-3"
onClick={onRequestClose}
/>
{onRequestClose && (
<CloseButton
size={ButtonSize.XSmall}
className="absolute right-1 top-1 laptop:right-3 laptop:top-3"
onClick={onRequestClose}
/>
)}

Copy link
Member

@sshanzel sshanzel left a comment

Choose a reason for hiding this comment

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

Great work! Thank you for your effort 🙏

After this, there should be two components we'll need to update to make this work on desktop. The MainComment and SubComment components contain a conditional render on the input field we display, we should pass the onRequestClose value to be a function that clears the state that manages the input itself.

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.

2 participants