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

FIDES-1429: Safari < 14 support #5410

Merged
merged 2 commits into from
Oct 23, 2024
Merged

FIDES-1429: Safari < 14 support #5410

merged 2 commits into from
Oct 23, 2024

Conversation

tvandort
Copy link
Contributor

@tvandort tvandort commented Oct 23, 2024

Closes FIDES-1429

Description Of Changes

The method we're using for media queries only became available as of Safari 14. Applied the work around mentioned in the stackoverflow post below to use an API that is available in Safari 13 when necessary.

https://stackoverflow.com/questions/62693995/addeventlistener-is-not-a-function-for-matchmedia-in-safari-browser/75517985#75517985

Code Changes

  • Add a fallback for media query event methods that works for Safari < 14

Steps to Confirm

  • Use browserstack to confirm that Safari < 14 allows the script to work.

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation:
    • documentation complete, PR opened in fidesdocs
    • documentation issue created in fidesdocs
    • if there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md
  • For API changes, the Postman collection has been updated
  • If there are any database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!

Copy link

vercel bot commented Oct 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
fides-plus-nightly ⬜️ Ignored (Inspect) Visit Preview Oct 23, 2024 6:12pm

@tvandort tvandort requested a review from gilluminate October 23, 2024 16:51
Copy link

cypress bot commented Oct 23, 2024

fides    Run #10589

Run Properties:  status check passed Passed #10589  •  git commit 0e5f65f23b ℹ️: Merge 9320554475b54402bae336aa6adc409485f9e8e3 into 17516d5c2527810be89c318fc2ef...
Project fides
Branch Review refs/pull/5410/merge
Run status status check passed Passed #10589
Run duration 00m 38s
Commit git commit 0e5f65f23b ℹ️: Merge 9320554475b54402bae336aa6adc409485f9e8e3 into 17516d5c2527810be89c318fc2ef...
Committer Tom Van Dort
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 4
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

if (matchQueryList.addEventListener) {
matchQueryList.addEventListener("change", handleChange);
} else {
// Supports Safari < 14
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest leaving this note out or updating it to be more generic. There's a good chance someone might see it and remove due to old browsers not being officially supported.

Suggested change
// Supports Safari < 14
// Older browser and test automation support

@tvandort tvandort merged commit 661347a into main Oct 23, 2024
13 checks passed
@tvandort tvandort deleted the FIDES-1429 branch October 23, 2024 18:35
Copy link

cypress bot commented Oct 23, 2024

fides    Run #10591

Run Properties:  status check passed Passed #10591  •  git commit 661347a700: FIDES-1429: Safari < 14 support (#5410)
Project fides
Branch Review main
Run status status check passed Passed #10591
Run duration 00m 41s
Commit git commit 661347a700: FIDES-1429: Safari < 14 support (#5410)
Committer Tom Van Dort
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 4
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

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