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

Vps 33/the invalid role back to scenarios button should be replaced #222

Conversation

jacobmathew105
Copy link
Contributor

Describe the issue

The invalid role page contains a button that redirects the user to a redundant homepage when intuitively clicking through the button.

Describe the solution

The “Back to scenarios” button on the Invalid Role page is replaced with a button saying “Try Access Scenario”. When Clicked, this button refreshes the page. The page contains the text: “Wait for your group member(s) to finish playing through their part of the scenario. Then, when it’s your turn to play through (your group members should let you know, click the below button: ”

Extra text as below is also added to the page: “If you have just finished playing your part of the scenario, let your group member with the {role with access} role know it’s their turn”.
image

Risk

The page may reload indefinitely when the button is clicked.

Definition of Done

  • Code peer-reviewed
  • Wiki Documentation is written and up to date
  • Unit tests written and passing
  • Integration tests written and passing
  • Continuous Integration build passing
  • Acceptance criteria met
  • Deployed to production environment

Reviewed By

Who reviewed your PR - for commit history once merged

git status Outdated Show resolved Hide resolved
@choden-dev
Copy link
Member

LGTM

Copy link
Contributor

@harbassan harbassan left a comment

Choose a reason for hiding this comment

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

Yep everything's working nicely 👍 (just remember when you're resolving the merge conflicts that the file is now in a different location)

Copy link
Member

@wjin-lee wjin-lee left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -22,27 +22,32 @@ function InvalidRolePage({ group }) {
fontWeight: "600",
display: "flex",
flexDirection: "column",
height: "100vh",
Copy link
Member

Choose a reason for hiding this comment

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

Not an issue, but on mobile, it's usually advised to consider using dvh if this was in any way targetted for mobile.

@jacobmathew105 jacobmathew105 force-pushed the VPS-33/The-Invalid-Role-Back-To-Scenarios-button-should-be-replaced branch 2 times, most recently from 231eadf to fc51b8b Compare August 26, 2024 10:58
@jacobmathew105 jacobmathew105 merged commit 6cc524e into master Aug 26, 2024
6 checks passed
@JordanBlenn JordanBlenn deleted the VPS-33/The-Invalid-Role-Back-To-Scenarios-button-should-be-replaced branch August 30, 2024 01:08
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.

4 participants