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 failing Magna Charta test in Safari #4661

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MartinJJones
Copy link
Contributor

@MartinJJones MartinJJones commented Feb 28, 2025

What

Fix failing Magna Charta test in Safari:

  • Updated the cW function to avoid truncating the calculated width, this matches the default behaviour in Magna Charta which does not restrict the number of decimal places
  • Use the toBeCloseTo matcher and set the number of decimal points to check to 4 when checking the calculated width

Why

Fixes: #2229

As mentioned in the GitHub issue issue there is a difference in the number of decimal places a browser will go to, using a chart from the component guide as an example:

Chrome

<div
  class="mc-td mc-bar-cell mc-bar-1 mc-bar-outdented"
  style="width: 2.70833%;" <- 5 decimal places
>
  <span style="margin-left: 100%; display: inline-block;">2</span>
</div>

Safari

<div
  class="mc-td mc-bar-cell mc-bar-1 mc-bar-outdented"
  style="width: 2.708333%;" <- 6 decimal places
>
  <span style="margin-left: 100%; display: inline-block;">2</span>
</div>

Further info

The failing test in Safari highlights a couple of other things we may want to consider in the future, for example:

  • We could update how widths are calculated in Magna Charta and restrict the number of decimal places so it is consistent across all browsers, unlikely to cause a noticeable difference visually when converted to pixels
  • There are lots of other failings tests in Safari, I've not investigated them further but I will create a new issue for this as it would be good to understand what is causing them and if they need to be fixed in Safari
  • Lastly, if the failing Safari tests in Jasmine highlight some differences/bugs in different browsers, we could consider running the tests in different browsers to help make them more visible

@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4661 February 28, 2025 12:36 Inactive
@MartinJJones MartinJJones changed the title Fix failing magnaCharta test in Safari Fix failing Magna Charta test in Safari Feb 28, 2025
- Updated the `cW` function to avoid truncating the calculated width, this matches the default behaviour in magnaCharta which does not restrict the number of decimal places
- Use the [toBeCloseTo](https://jasmine.github.io/api/5.6/matchers#toBeCloseTo) matcher and set the number of decimal points to check to 4

Fixes: #2229
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.

Magna charta test failing in Safari 14.1.1
2 participants