Skip to content

feat: [UEPR-190] Connecting ToU modal UI to backend #9292

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

Open
wants to merge 7 commits into
base: ux-integration
Choose a base branch
from

Conversation

Bogomil-Stoyanov
Copy link
Contributor

@Bogomil-Stoyanov Bogomil-Stoyanov commented Mar 6, 2025

Ticket:
UEPR-190

Error UI:
image

@Bogomil-Stoyanov Bogomil-Stoyanov changed the base branch from ux-integration to develop March 6, 2025 09:59
@Bogomil-Stoyanov Bogomil-Stoyanov changed the base branch from develop to ux-integration March 6, 2025 09:59
Comment on lines 73 to +75
return (<TermsOfUseModalUnder16
isOpen={isModalVisible}
onClose={handleClose}
onAccept={handleAccept}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we not passing showError to TermsOfUseModalUnder16?

@MiroslavDionisiev
Copy link
Contributor

There is an empty file error-msg.jsx

@@ -19,16 +20,33 @@ export const TermsOfUseLink = chunks => (
</a>
);


Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: remove extra line

@@ -47,13 +65,14 @@ const TermsOfUseModal = ({
if (!usesParentEmail || isOver16){
return (<TermsOfUseModalOver16
isOpen={isModalVisible}
onClose={handleClose}
onAccept={handleAccept}
showErrorMessage={showError}
/>);
}

return (<TermsOfUseModalUnder16
Copy link
Contributor

@KManolov3 KManolov3 Mar 6, 2025

Choose a reason for hiding this comment

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

Since we're showing the same error message in both cases, what do you think about showing it here, in the parent component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It needs to be in the modal itself. I can try changing it so that we have a single Modal element and only modify the content based on the age group. Initially, I took this approach because the modals behaved differently (e.g., showing the close button, being dismissible, etc.), but since we no longer have such functionality, we might as well use a single modal component and only change its contents.

@@ -61,22 +61,7 @@ module.exports.requestSessionWithRetry = (resolve, reject, retriesLeft, totalDel
);
}
console.log('session body:', body);
Copy link
Contributor

Choose a reason for hiding this comment

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

console.log

Copy link
Contributor

Choose a reason for hiding this comment

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

Empty file

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.

3 participants