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

fix(form-v2): implement full list numbering for TOU and PP pages #4424

Merged
merged 14 commits into from
Aug 2, 2022

Conversation

karrui
Copy link
Contributor

@karrui karrui commented Jul 29, 2022

This PR is built on #4259, and supersedes that PR.

Problem

Numbering in the react version's terms of use page and privacy policy page is inconsistent with angular JS app as outlined in #4111 .

Closes [#4111 ]

Solution

This PR resolves this issue by implementing a customizable reusable component which is able to perform nested listing with the correct sequence marker syntax intended. Refer to the following links for a more comprehensive understanding
https://developer.mozilla.org/en-US/docs/Web/CSS/counter
https://developer.mozilla.org/en-US/docs/Web/CSS/counters
https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Counter_Styles/Using_CSS_counters

Breaking Changes

  • Yes - this PR contains breaking changes
    • Details ...
  • No - this PR is backwards compatible

Features:

  • refer to pictures

Improvements:

  • terms of use page and policy page had components which were basically clones of each other. I refactored them into reusable components and removed the redundant code.

Before & After Screenshots

BEFORE:

AFTER:

terms of use page

terms_of_use_page_after

terms of use page with css borders

terms_of_use_page_after_with_css

privacy page

privacy_page_after

privacy page with css borders

privacy_page_after_with_css

Tests

  • terms of use and policy don't need any unit test as they are purely cosmetic

@karrui karrui requested a review from mantariksh July 29, 2022 04:57
Copy link
Contributor

@wanlingt wanlingt left a comment

Choose a reason for hiding this comment

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

lgtm!

Copy link
Contributor

@mantariksh mantariksh left a comment

Choose a reason for hiding this comment

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

gtlm

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.

5 participants