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

yihan hide assign badges button from all but owner/admin class #771

Merged
merged 2 commits into from
Apr 17, 2023

Conversation

AriaYu927
Copy link
Contributor

@AriaYu927 AriaYu927 commented Apr 14, 2023

Description

image

Mainly changes explained:

Add conditional rendering on assign badges button. So, the button only shows up when the users is of Owner/Admin class or has assign badges permission.

How to test:

check into current branch
do npm install and ... to run this PR locally
log as different type of user
go to profile page→ Assign badges→ Choose→ Save
check if only owner, administrator and users who has the permission can see the button and assign badges.

Screenshots or videos of changes:

owner, administrator or other users who have assign badges permission:
hide-button-has-permission

users don't have assign badges permission:
hide-button-no-permission

@LucileTech
Copy link
Member

Hit @AriaYu927 I tested your PR with Volunteer, Admin and Core Team rights and it works as intended, the Assign Badge button is only displayed with Admin Rights:

Screenshot 2023-04-14 at 18 10 08

Screenshot 2023-04-14 at 18 10 58

Screenshot 2023-04-14 at 18 11 31

Copy link
Contributor

@abdel-lall abdel-lall left a comment

Choose a reason for hiding this comment

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

worked for me. good job!

771(1)
771(2)

Copy link
Contributor

@PrabhjotSembhi PrabhjotSembhi left a comment

Choose a reason for hiding this comment

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

it has issues please check my video

HGN.APP.-.Google.Chrome.2023-04-15.15-44-25.mp4

You can fix this or much better will redo it on a new branch

@KaixiangGU
Copy link
Contributor

It works on my end, check the video below. Thank you @AriaYu927

bandicam.2023-04-15.10-57-26-532.mp4

@AriaYu927 AriaYu927 closed this Apr 15, 2023
@AriaYu927 AriaYu927 reopened this Apr 15, 2023
@AriaYu927
Copy link
Contributor Author

it has issues please check my video

You can fix this or much better will redo it on a new branch

Hi @PrabhjotSembhi , thank you for pointing out the problem! I tested it on my end but I wasn't able to reproduce the issue. Here's the video:

PR_771.mp4

@AriaYu927
Copy link
Contributor Author

It works on my end, check the video below. Thank you @AriaYu927

Thank you for testing @KaixiangGU

@one-community
Copy link
Member

Don't close PRs if you can avoid it, fix them instead.

Copy link
Contributor

@ypan710 ypan710 left a comment

Choose a reason for hiding this comment

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

Hi @AriaYu927, I've tested your PR and it worked as expected.

Screen Shot 2023-04-15 at 1 42 27 PM
Screen Shot 2023-04-15 at 1 42 13 PM
Screen Shot 2023-04-15 at 1 41 48 PM
Screen Shot 2023-04-15 at 1 40 27 PM
Screen Shot 2023-04-15 at 1 40 11 PM

Copy link
Contributor

@Lucas-E Lucas-E left a comment

Choose a reason for hiding this comment

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

Hey Yihan, just tested you PR and it works as expected!
Captura de tela de 2023-04-15 19-27-52

@one-community
Copy link
Member

Thank you all, will merge this as soon as Prabhjot approves it too. Following up with him by DM.

@PrabhjotSembhi PrabhjotSembhi dismissed their stale review April 17, 2023 04:53

I found this I am facing this issue in development branch. I want others to check this so we can fix this bug if it this bug persist.

@one-community one-community merged commit e5a0503 into development Apr 17, 2023
@EvianTan EvianTan deleted the yihan-hide-assign-badges-button branch June 23, 2023 20:52
@AriaYu927 AriaYu927 mentioned this pull request Aug 16, 2023
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.

8 participants