18F / pages

DEPRECATED: Publishing platform for 18F sites a la GitHub pages
https://pages.18f.gov
Other
63 stars 17 forks source link

Side navigation accordion functionality disabled when not on pages.18f.gov/guides-template/ #53

Open juliaelman opened 8 years ago

juliaelman commented 8 years ago

The side navigation works when you are at the following url only: https://pages.18f.gov/guides-template/ see recordings of this bug.

Poking through the repo a bit, it could possibly be fixed by changing the following to point to guides-template instead of /guides-template. See the following lines for reference:

There might be more to it than this and point to @mbland for guidance on best resolves.

WORKING dmde1itqqi

NOT WORKING humpbr6uun

mbland commented 8 years ago

It's actually something else I haven't sat down to figure out yet:

Uncaught ReferenceError: jQuery is not defined
(anonymous function) @ jquery-migrate-1.2.1.min.js:2

Uncaught ReferenceError: $ is not defined
(anonymous function) @ accordion.js:49

Try forcing a reload in your "not working" case and you'll see the errors go away...for a little bit.

Looks like some kind of jQuery loading failure. Ring any bells?

juliaelman commented 8 years ago

@mbland ah yes! I saw this in the console too, but wasn't 100% sure if it was an asset reference issue.

Is there a reason why async is being added to these <script> tags? Not sure if those are needed or not and might help resolve the issue too:

https://github.com/18F/guides-style/blob/master/lib/guides_style_18f/includes/scripts.html

mbland commented 8 years ago

Possibly... Wondering if there's a lesson here about "proper" async loading, as I'd like to keep that functionality if possible. Here's a couple quick searches I'll look at more closely tomorrow:

juliaelman commented 8 years ago

@mbland cool and thanks for those articles! Based on these suggestions, I am thinking that we can possibly remove the async attribute. Since the scripts are loading at the bottom of the page, it might not be necessary to also attach this attribute to the <script> tag. This may also help resolve the issue, but would love to know your thoughts prior to submitting a pull request.

mbland commented 8 years ago

D'oh! Of course. Feel free to file an issue against 18F/guides-style; the file you need is lib/guides_style_18f/includes/scripts.html. Also, we'll need to cut a new guides_style_18f gem from that repo and update each guide manually (still haven't figured out how to automate that part).

juliaelman commented 8 years ago

:+1: on it!

wslack commented 8 years ago

Should this be closed?

juliaelman commented 8 years ago

@wslack not quite? AS @mbland mentioned (https://github.com/18F/pages/issues/53#issuecomment-161988567), a new gem needs to be but and all guides will need to be updated to have this change take true effect.

wslack commented 8 years ago

ah kk