Skip to content

Conversation

@binh-dam-ibigroup
Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup commented Mar 8, 2021

This PR defines two new optional, custom component entries in the components context that are required for OTP middleware implementations:

  • TermsOfService (accessed from the "terms of service..." checkbox in the user account page)
  • TermsOfStorage(accessed from the "terms of storage" checkbox in the user account page)

The PR implements the routes and wrappers to display these components.
(The contents of these components is out of scope.)

Copy link
Member

@landonreed landonreed left a comment

Choose a reason for hiding this comment

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

Looks good! But I think there's some extra abstraction in here that may not be needed.

<AppFrame>
{Component
? <Component />
: <p>No content provided.</p>
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should create a PageNotFound component and render that if Component does not exist (it could have a link back to the main trip planner).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm I have set these pages to open in new tabs, so does a link make sense with this setup?

Copy link
Member

Choose a reason for hiding this comment

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

Yea, perhaps no link in that case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I enhanced the format for not found content in f956388.

Copy link
Member

@landonreed landonreed left a comment

Choose a reason for hiding this comment

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

OK yep, thanks for clarifying that need for AppFrame. Only one non-blocking comment about creating/using a PageNotFound page that gives a little more help to the user to get back to the main trip planner.

@binh-dam-ibigroup binh-dam-ibigroup removed their assignment Mar 8, 2021
Copy link
Contributor

@evansiroky evansiroky left a comment

Choose a reason for hiding this comment

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

Good to go pending merge conflict resolution

@binh-dam-ibigroup binh-dam-ibigroup merged commit 34eab65 into dev Mar 12, 2021
@binh-dam-ibigroup binh-dam-ibigroup deleted the terms-conditions branch March 12, 2021 19:06
@evansiroky evansiroky mentioned this pull request Mar 31, 2021
@github-actions
Copy link
Contributor

🎉 This PR is included in version 3.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants