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

chore(payments): Missing markup in payment-confirm string #12875

Merged
merged 1 commit into from
May 17, 2022
Merged

Conversation

annasob
Copy link
Contributor

@annasob annasob commented May 13, 2022

Because

  • This variable is not being used anywhere. The linter is broken on localization files.

This pull request

  • Removes the unused variable

Issue that this pull request solves

Closes: #12726

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

Any other information that is important to this pull request.

@annasob annasob requested a review from a team as a code owner May 13, 2022 18:42
@annasob annasob requested a review from StaberindeZA May 13, 2022 18:42
Copy link
Contributor

@StaberindeZA StaberindeZA left a comment

Choose a reason for hiding this comment

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

r+ wc.

@sardesam There's some small changes on the tests I was hoping you could make, but otherwise looks good.

expect(
legalCheckbox.props.children.props.children[1].props.children
).toMatch(
/\$\d+[.]\d{2}[ ][daily|every 6 days|weekly|every 6 weeks|monthly|every 6 months|yearly|every 6 years]/
Copy link
Contributor

Choose a reason for hiding this comment

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

This would work, however it runs the risk of returning a false positive. (If for some reason, as a bug, formatPlanPricing returns "daily" regardless of input, all these tests will still pass, even though there's a bug)

This shouldn't hold up the change though, so feel free to squash and merge.

@StaberindeZA StaberindeZA changed the title Fxa 5015 chore(payments): Missing markup in payment-confirm string May 17, 2022
@sardesam sardesam merged commit 300b945 into main May 17, 2022
@sardesam sardesam deleted the FXA-5015 branch May 17, 2022 22:40
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.

Missing markup in payment-confirm string
3 participants