Fix failing Magna Charta test in Safari

[MartinJJones]

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

[jon-kirwan]

Just a small note - do we need two expectations here? Perhaps we should merge them into a single test that checks the correct width as a percentage string?

[MartinJJones]

Agreed, I think this would be the ideal approach, I cannot see a good way to do this however while supporting both Safari and Chrome. For example, Magna Charta does not currently set the number of decimal places, using 2.7083333333 as an example width, this will get set as below in each browser:

Chrome = 2.70833%
Safari = 2.708333%

We have a couple of options to get around this:

Update the widths before doing the comparison

I've added an example of this in the latest commit - 2f6b5f7.

I'm not 100% sure about this approach though, especially as it updates the width set in the browser, using the toBeCloseTo matcher feels better here I think.

Update Magna Charta

Another option could be to update Magna Charta so it does restrict the number of decimal places, ensuring a consistent width in each browser, overall this feels like it would be done to satisfy the tests and seems unlikely to provide any benefits, could be worth exploring further.

Does the issue need fixing?

Lastly, I think even if we do update the tests to support Safari, we are unlikely to see the errors unless we test this locally as we use headlessChrome.

I would be reluctant to dismiss it completely until we can be certain that the other failing tests are not genuine issues in Safari, but does raise the question of how we could make failures in Safari more visible in the future if required.

[jon-kirwan]

Hi @MartinJJones, sorry to take a while to respond and thanks for adding the extra commit. I see what you mean and agree with you regarding the above options.

I think then your first fix is probably the best approach:

expect(cellWidth).toContain('%')
expect(parseFloat(cellWidth)).toBeCloseTo(calculatedWidth, 4)

Two final things: perhaps it's worth raising it again at the community meeting to see if we should fix Jasmine Safari issues? Or whether it's better to only acknowledge the issue.

One final idea we could try is to match a Regex which should allow for one expectation to check for the correct value in both Chrome and Safari and the appended percentage character e.g.

var cellStyleWidth = cell.style.width
var calculatedWidth = parseFloat(cW(12, cell.textContent))
// allows up to 2 more digits after the main decimal point to accommodate Safari's rounding behaviour
var regex = new RegExp(`^${calculatedWidth}(\\d{0,2})?%$`);

expect(cellStyleWidth).toMatch(regex);