Skip to content
This repository was archived by the owner on Mar 1, 2025. It is now read-only.

Conversation

@prankurpandeyy
Copy link

@prankurpandeyy prankurpandeyy commented Nov 1, 2022

closes : #466

Issue :

The cursor is not pointer when super user hovers over new user profile .

Files Changed

index.js
new-members.module.scss

Fixes :

Added the cursor pointer when the superuser hovers on new user profiles.
For better understanding and more reference, Please read the issue ticket: #466

Issue

Screenshot from 2022-11-07 21-41-49

@vercel
Copy link

vercel bot commented Nov 1, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
website-members ✅ Ready (Inspect) Visit Preview Nov 10, 2022 at 3:37AM (UTC)

@Maheima
Copy link

Maheima commented Nov 1, 2022

If this condition is just for super user then, why do other user able to see the pointer icon too?
Also, in the description of the PR , These are pointers for you to fill in when sending a PR. If there is nothing to mention in them, remove them.
image

@prankurpandeyy
Copy link
Author

@Maheima I have fixed the issues which you pointed out yesterday , you can review it.

}
}
.containerForNewMemberHover {
@extend .containerForNewMember;
Copy link
Contributor

Choose a reason for hiding this comment

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

Impressive 🙌
Good use of @extend

Copy link
Author

@prankurpandeyy prankurpandeyy Nov 2, 2022

Choose a reason for hiding this comment

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

Thanks for the review.


.newUser {
pointer-events: none;
pointer-events: auto;
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the expectation, only super user should perform some action on pointer event and rest shouldn't?
Clarify in case I am thinking wrong here

Copy link
Author

@prankurpandeyy prankurpandeyy Nov 2, 2022

Choose a reason for hiding this comment

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

Yes anyone with a super user role should be able to have a cursor pointer property for the rest of the users it should be the default cursor pointer.

<div
className={
isSuperUser
? styles.containerForNewMemberHover
Copy link

Choose a reason for hiding this comment

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

Can you please think of better classnames here? Rest everything looks great!

Copy link
Author

Choose a reason for hiding this comment

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

Sure, I am changing it to something more meanigful.

Copy link

@Maheima Maheima left a comment

Choose a reason for hiding this comment

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

LGTM! Just resolve all the conversations

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cursor should be pointer when the super hover over users

4 participants