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] Pin icon should not be visible to users without pin permission (revised) #549

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

thesynthax
Copy link
Contributor

Brief Title

Pin icon should not be visible to users without the pin permission like in main Rocket.Chat application.
Check #518, #519 and #520 for complete details

Acceptance Criteria fulfillment

  • Pin Icon is now not visible to users not having Pin Permissions.

Fixes #519

Video/Screenshots

ss_146

P.S: This PR is a revision of #520 after #518 is merged. Following this, #520 will be discarded.

@thesynthax
Copy link
Contributor Author

@Spiral-Memory @sidmohanty11 please have a look.

userRoles.length > 0 &&
postMsgRoles.some((role) => userRoles.includes(role));
setCanSendMsg(canSendMsg);
const concatenatedRoles = channelMemberRoles.concat(
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can use shorthand here, destructure and combine. I have raised a PR feat: Addition of chatinfo to display status, have a look here..

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @Spiral-Memory, can you share that pr number?

Copy link
Contributor

@devanshkansagra devanshkansagra Oct 19, 2024

Choose a reason for hiding this comment

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

Hey @Spiral-Memory, if we destructure and combine, the users are unable to login

@@ -235,6 +235,7 @@ const Message = ({
{!message.t && showToolbox ? (
<MessageToolbox
message={message}
RCInstance={RCInstance}
Copy link
Collaborator

@Spiral-Memory Spiral-Memory Apr 11, 2024

Choose a reason for hiding this comment

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

Why is this added ? Any specific reason?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @Spiral-Memory the adding of RCInstance as a prop was unnecessary, after removing it I have seen no change

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.

[Feature Request] Pin icon should not be visible to users without pin permissions
3 participants