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: correct alignment issues in preview mode #7109

Merged
merged 2 commits into from
Feb 22, 2024

Conversation

kathleenkhy
Copy link
Contributor

@kathleenkhy kathleenkhy commented Feb 22, 2024

Problem

There were alignment issues found where the text would bleed out of the page and container if the text is too long.

Root cause: Originally, the width of the Stack component in PaymentEndPagePreview.tsx was not indicated to be 100%, which might have caused the text to bleed since no width was defined. In the Edit mode and the actual Thank You Page (which worked as intended and the text did not bleed), the widths were defined as 100%.

image

Closes FRM-1656

Solution

  • Fixed alignment and width of the component such that the text does not bleed beyond the page
  • Fixed padding of components in mobile view
  • Fixed padding of preview banner warning

Breaking Changes

  • No - this PR is backwards compatible

Before & After Screenshots

BEFORE:

Mobile View

image

AFTER:

Desktop View

image

Mobile View

image

Tests

  • Add a payment variable to a form
  • Edit the Thank You page such that the title and follow-up instructions are very long
  • Click on the Preview (eye) button of the form in admin mode to enter Preview mode
  • The title and follow up instructions text (which are very long) should still be within the Summary table
  • In mobile view, the preview banner instructions and components should have sufficient padding

Copy link

linear bot commented Feb 22, 2024

@kathleenkhy kathleenkhy marked this pull request as ready for review February 22, 2024 06:14
@KenLSM
Copy link
Contributor

KenLSM commented Feb 22, 2024

@kathleenkhy could we have some commentary, added on the Problem Section of the PR message, on how this occurred (a mini-root cause analysis).

Was it a regression introduced by the previous PR? i.e.:

  • it was a new component introduced in the previous PR that didn't have the correct styles?
  • existing issue?

@KenLSM
Copy link
Contributor

KenLSM commented Feb 22, 2024

@kathleenkhy Sorry for being so anal on your PRs -- we don't usually do a full RCA on the PR itself, but since these are your first few PRs, we want to be a little more hands-on to make sure that we're taking away lessons whenever possible.

@kathleenkhy kathleenkhy force-pushed the fix/payments-thank-you-preview branch from 934044e to 95fd061 Compare February 22, 2024 06:51
@kathleenkhy
Copy link
Contributor Author

kathleenkhy commented Feb 22, 2024

@KenLSM no worries and thanks for the feedback 🙂! I added the finding I had in the problem section (I didn't define the width as 100% which caused the text to bleed)! Lmk if I should add more details!

@kathleenkhy kathleenkhy force-pushed the fix/payments-thank-you-preview branch from 95fd061 to 339c64d Compare February 22, 2024 06:56
@kathleenkhy kathleenkhy requested a review from wanlingt February 22, 2024 07:00
@kathleenkhy kathleenkhy force-pushed the fix/payments-thank-you-preview branch from 339c64d to 136c334 Compare February 22, 2024 09:38
Copy link
Contributor

@KenLSM KenLSM left a comment

Choose a reason for hiding this comment

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

LGTM! (did pair programming to reduce the changes required)

@kathleenkhy kathleenkhy merged commit 7e1b9c8 into develop Feb 22, 2024
21 of 22 checks passed
@kathleenkhy kathleenkhy deleted the fix/payments-thank-you-preview branch February 22, 2024 09:57
@KenLSM KenLSM mentioned this pull request Mar 4, 2024
26 tasks
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.

2 participants