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

Safari NumberFormat error fix. #3289

Merged
merged 3 commits into from
Jul 23, 2021

Conversation

revanth0212
Copy link
Contributor

Description

Safari browser has a bug related to the INTL NumberFormat polyfill. We are already handling it in the code but unfortunately, we are throwing an error as well. Since an error is thrown the whole app crashes on Safari instead of rendering the app with the handled logic.

This PR simply changes the function to not throw and warn instead to let the backup logic do its magic and render the app.

magento.pwa-venia.com
image

develop.pwa-venia.com
image

After Fix
image

Related Issue

Closes PWA-1894

Acceptance

App should render properly on Safari.

Verification Stakeholders

@eug123
@tjwiebell

Verification Steps

  1. Open the deployed app in the safari browser. App should render properly.
  2. Go to any category page and prices should render properly.
  3. Switch between locales and currencies and nothing should be broken.

Is Browser/Device testing needed?

Yes, Browser testing is needed.

Breaking Changes (if any)

None

Checklist

  • I have added tests to cover my changes, if necessary.
  • I have added translations for new strings, if necessary.
  • I have updated the documentation accordingly, if necessary.

@revanth0212 revanth0212 added the version: Minor This changeset includes functionality added in a backwards compatible manner. label Jul 21, 2021
@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Jul 21, 2021

Messages
📖

Associated JIRA tickets: PWA-1894.

📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

Generated by 🚫 dangerJS against 31c9dc0

Copy link
Contributor

@tjwiebell tjwiebell left a comment

Choose a reason for hiding this comment

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

Good find, fix works perfectly 👍

@supernova-at
Copy link
Contributor

supernova-at commented Jul 23, 2021

QA

📝 Tested on using Safari Version 14.1.2 (14611.3.10.1.5) on macOS Mojave 10.14.6.

To Do

✅ Passed

  • Test Plan
  • MFTF
  • Lighthouse
  • Alternate Browsers

🚫 Failed

  • Cypress

PageBuilder button and image tests kept failing but this change had nothing to do with them. The backend is quite slow currently so I am merging even though I haven't actually witnessed these tests pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:venia-ui Progress: done version: Minor This changeset includes functionality added in a backwards compatible manner.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants