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

Resolve IE11 accordion gem doc duplication #2036

Merged
merged 2 commits into from
Apr 28, 2021
Merged

Conversation

chris-gds
Copy link
Contributor

@chris-gds chris-gds commented Apr 27, 2021

What?

Updating to use polyfill version for forEach for IE11

Why?

The Accordion on the gem docs was being visually duplicated.

Visual Changes

Before
image

After

Screenshot 2021-04-28 at 09 03 02

Anything else?

"querySelectorAll() is a method which return a NodeList. And on Internet Explorer, foreach() only works on Array objects. (It works with NodeList with ES6, not supported by IE11)." ref

Related PR

The Accordion on the gem docs was being visually duplicated - updating to use polyfill version for forEach for IE11.
@bevanloon bevanloon temporarily deployed to govuk-publis-resolve-ie-ppfoyd April 27, 2021 19:21 Inactive
@chris-gds chris-gds changed the title Resolve IE11 accordion gem duplication Resolve IE11 accordion gem docs duplication Apr 27, 2021
@chris-gds chris-gds changed the title Resolve IE11 accordion gem docs duplication Resolve gem docs duplication Apr 27, 2021
@chris-gds chris-gds changed the title Resolve gem docs duplication Resolve IE11 accordion gem doc duplication Apr 27, 2021
@bevanloon bevanloon temporarily deployed to govuk-publis-resolve-ie-ppfoyd April 27, 2021 19:27 Inactive
@chris-gds chris-gds requested review from owenatgov and maxgds April 28, 2021 08:05
@chris-gds chris-gds marked this pull request as ready for review April 28, 2021 08:06
@chris-gds chris-gds added bug and removed bug labels Apr 28, 2021
Copy link
Contributor

@owenatgov owenatgov left a comment

Choose a reason for hiding this comment

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

LOL whoops 😂 Amazing work debugging this Chris. Would it be possible to add some notes in the PR Why going into more detail about why this was happening and why the fix solves it?

@maxgds
Copy link
Contributor

maxgds commented Apr 28, 2021

LOL whoops 😂 Amazing work debugging this Chris. Would it be possible to add some notes in the PR Why going into more detail about why this was happening and why the fix solves it?

Yeah, I'm really puzzled. I have been testing this on browserstack and it looks solid as a fix but I'm baffled if that is the fix as to why the accordions on the live site weren't affected but in the component guide they were. Do you have any insight, Chris?

@chris-gds
Copy link
Contributor Author

chris-gds commented Apr 28, 2021

@owenatgov course - I'll update here and in the PR notes.

"querySelectorAll() is a method which return a NodeList. And on Internet Explorer, foreach() only works on Array objects. (It works with NodeList with ES6, not supported by IE11)." ref

Essentially the Design System version had a polyfill in place that was ported over, this just needed to be updated to use it. I remember facing a similar issue with this when initial work started (missed spotting this at review for 1937 🙈)

why the accordions on the live site weren't affected but in the component guide they were.

It's a good question @maxgds the issues flags on this page within IE however the resulting duplication doesn't look present, I would need to spend some time investigating my assumption is the differences in what's loaded on live vs the gem docs.

@chris-gds chris-gds merged commit 9f7af5a into master Apr 28, 2021
@chris-gds chris-gds deleted the resolve-ie11-duplicate branch April 28, 2021 10:30
@injms injms mentioned this pull request Apr 28, 2021
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.

4 participants