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

feat: Add Unpin and Unstar message functionality to the icons in Message Headers #796

Conversation

AyushKumar123456789
Copy link

@AyushKumar123456789 AyushKumar123456789 commented Jan 4, 2025

Brief Title

Acceptance Criteria fulfillment

  • Task 1 : Implement “unpin” from the pinned files list by clicking the pin icon.
  • Task 2 : Implement “unstar” from the starred files list by clicking the star icon.
  • Task 3 : Copy direct link to message and Extra menu toolbox is not implemented , (To keep UI minimal , because Embedded chat is often nested inside some other application).

Fixes #741

In this PR, I’ve opted for a simpler approach: users can unpin or unstar a file directly by clicking the existing icons. This preserves a clean interface within EmbeddedChat, which is often nested in other applications. Adding a separate “Copy Link” or extra menu might complicate the embedded experience.

Video/Screenshots

unpin.and.unstar.mp4

PR Test Details

Note: The PR will be ready for live testing at https://rocketchat.github.io/EmbeddedChat/pulls/pr-<pr_number> after approval. Contributors are requested to replace <pr_number> with the actual PR number.

@abirc8010
Copy link
Contributor

Hey @AyushKumar123456789 I think this is already covered in this PR #744

@AyushKumar123456789
Copy link
Author

Yes sir, but my implementation was different keeping in mind that embedded chat is itself nested in some other components, So in my opinion implementation of toolbox menu might increase unnecessary complexity which is that PR.

@abirc8010
Copy link
Contributor

Adding a separate “Copy Link” or extra menu might complicate the embedded experience.

I'm not quite sure how this would complicate the embedded experience. Let @Spiral-Memory share his thoughts.

Also you can call me Abir no need of "Sir" 🙂

@AyushKumar123456789
Copy link
Author

AyushKumar123456789 commented Jan 5, 2025

Thank you for input Abir, Maybe you were right about ,

I'm not quite sure how this would complicate the embedded experience. Let @Spiral-Memory share his thoughts.

If that the case you can close the PR, I also made another PR #776 , can you please give me feedback on that
Thank You

@Spiral-Memory
Copy link
Collaborator

Hey @AyushKumar123456789
Thanks for this PR.. but i think its better we follow how Rocket.Chat manages it.. I understand the concern being it embedded, but still i don't feel it will complicate experience

Thanks

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.

Feat: Enhance Starred and Pinned Messages Management with Unstar, Unpin, Copy Link, and Navigate Options
3 participants