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

Created a popover for adding links to editor content #904

Merged
merged 9 commits into from
Oct 17, 2023

Conversation

gaagul
Copy link
Contributor

@gaagul gaagul commented Oct 6, 2023

Description

  • Added: popover to add links to editor

Checklist

- [ ] I have made corresponding changes to the documentation.
- [ ] I have updated the types definition of modified exports.

  • I have verified the functionality in some of the neeto web-apps.
  • I have added the necessary label (patch/minor/major - If package publish
    is required).

Reviewers

@AbhayVAshokan _a

@gaagul gaagul added the patch Releases small requests or bug fixes. label Oct 6, 2023
@unnitallman unnitallman temporarily deployed to neeto-editor-pr-904 October 6, 2023 10:08 Inactive
@unnitallman unnitallman temporarily deployed to neeto-editor-pr-904 October 7, 2023 11:53 Inactive
@unnitallman unnitallman temporarily deployed to neeto-editor-pr-904 October 9, 2023 09:27 Inactive
@bot-bigbinary
Copy link
Contributor

Cannot auto-update because of conflicts.

Copy link
Member

@AbhayVAshokan AbhayVAshokan left a comment

Choose a reason for hiding this comment

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

@gaagul _a

There are a few issues with this approach:

  • Pressing enter should submit the form. Instead, it is replacing the text content.
  • Add more spacing between the two inputs and the buttons (use space-y-2).
  • Update the label of the link button to Done.
  • The placement of the popover is wrong. So it's more confusing.
  • Use the useOnClickOutside hook from neeto-commons-frontend to close the popover when the user clicks outside.

@neetogit-bot neetogit-bot bot assigned gaagul and unassigned AbhayVAshokan Oct 9, 2023
@unnitallman unnitallman temporarily deployed to neeto-editor-pr-904 October 16, 2023 18:10 Inactive
@unnitallman unnitallman temporarily deployed to neeto-editor-pr-904 October 16, 2023 18:17 Inactive
@gaagul
Copy link
Contributor Author

gaagul commented Oct 16, 2023

@AbhayVAshokan _a Please review the new changes to UI.

@neetogit-bot neetogit-bot bot assigned AbhayVAshokan and unassigned gaagul Oct 16, 2023
Copy link
Member

@AbhayVAshokan AbhayVAshokan left a comment

Choose a reason for hiding this comment

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

@neetogit-bot neetogit-bot bot assigned gaagul and unassigned AbhayVAshokan Oct 17, 2023
@gaagul
Copy link
Contributor Author

gaagul commented Oct 17, 2023

@gaagul _a https://vimeo.com/875137202/aeb4404134?share=copy

I think the mic was on mute while you were recording.

@AbhayVAshokan
Copy link
Member

@gaagul: I have recorded it again: https://vimeo.com/875156777/1072dfa383?share=copy

@unnitallman unnitallman temporarily deployed to neeto-editor-pr-904 October 17, 2023 13:19 Inactive
@gaagul
Copy link
Contributor Author

gaagul commented Oct 17, 2023

@AbhayVAshokan _a I have fixed the issues and verified the changes, could you please verify it once more to find out whether I have missed any edge cases.

@neetogit-bot neetogit-bot bot assigned AbhayVAshokan and unassigned gaagul Oct 17, 2023
@AbhayVAshokan
Copy link
Member

@gaagul _a in order to close the popover I need to click within the editor itself. I need to close it by clicking it anywhere.

image

@neetogit-bot neetogit-bot bot assigned gaagul and unassigned AbhayVAshokan Oct 17, 2023
@gaagul
Copy link
Contributor Author

gaagul commented Oct 17, 2023

@gaagul _a in order to close the popover I need to click within the editor itself. I need to close it by clicking it anywhere.

@AbhayVAshokan I will merge the PR myself after fixing this outsideClick issue if everything else looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Releases small requests or bug fixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issues with links
4 participants