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.

Sign up for GitHub

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

Jump to bottom

feat(install-modal): block access code usage until user is logged in #1182

Merged
merged 8 commits into from
Dec 10, 2024
Merged

feat(install-modal): block access code usage until user is logged in #1182

merged 8 commits into from
Dec 10, 2024

Conversation

eliobricenov
Copy link
Contributor

@eliobricenov eliobricenov commented Dec 5, 2024 •
edited
Loading

<--- Put the description here --->

closes https://github.com/HyperPlay-Gaming/product-management/issues/719

How to test

  1. Add a game that requires access code (Revolon or Market Wars for example) to your library
  2. Click install
  3. Check that the login alert error is displayed if you're not logged in. On Market Wars it shows in the step 4 content.
    image
    image
    image
  4. Check that the modal update shows the login alert too
    image

Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@eliobricenov eliobricenov self-assigned this Dec 5, 2024
@eliobricenov eliobricenov added the PR: Ready-For-Review PR is ready to be reviewed by peers label Dec 5, 2024
BrettCleary
BrettCleary approved these changes Dec 6, 2024
View reviewed changes
Copy link
Collaborator

@BrettCleary BrettCleary left a comment

Choose a reason for hiding this comment

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

LGTM

src/frontend/screens/Library/components/InstallModal/index.tsx Outdated
Comment on lines 211 to 219
<AlertCard
showClose={false}
title={t('installModal.loginRequired', 'Login Required')}
message={t(
'installModal.loginRequiredMessage',
'You need to be logged into HyperPlay to enter your access code and install this game. '
)}
variant="warning"
/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we make this a shared component to reuse in src/frontend/components/UI/GameUpdateDialog/index.tsx?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup, good call

flavioislima
flavioislima approved these changes Dec 6, 2024
View reviewed changes
Copy link
Contributor

@flavioislima flavioislima left a comment

Choose a reason for hiding this comment

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

Good stuff, hermano!

@eliobricenov eliobricenov added PR: Ready-For-Test PR is ready to be tested by a QA and removed PR: Ready-For-Review PR is ready to be reviewed by peers labels Dec 9, 2024
nyghtstalker
nyghtstalker approved these changes Dec 9, 2024
View reviewed changes
Copy link

@nyghtstalker nyghtstalker left a comment

Choose a reason for hiding this comment

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

  • Confirmed that the alert is not displayed when the user is logged in
  • Confirmed that the alert is displayed when the user is not logged in
  • Confirmed that the alert is displayed when the user switches to a game branch that does require a code but main does not

image
image
image
image
image
image
image

@eliobricenov eliobricenov merged commit eabfabf into main Dec 10, 2024
11 checks passed
@eliobricenov eliobricenov deleted the feat/require-login-access-code branch December 10, 2024 18:34
@github-actions github-actions bot locked and limited conversation to collaborators Dec 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@BrettCleary BrettCleary BrettCleary approved these changes

@nyghtstalker nyghtstalker nyghtstalker approved these changes

@flavioislima flavioislima flavioislima approved these changes

@jiyuu-jin jiyuu-jin jiyuu-jin approved these changes

Assignees

@eliobricenov eliobricenov

Labels
PR: Ready-For-Test PR is ready to be tested by a QA
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@eliobricenov @jiyuu-jin @flavioislima @BrettCleary @nyghtstalker