18F / brand

18F Brand
https://brand.18f.gov
Other
27 stars 21 forks source link

Styleguide updates #177

Closed Dahianna closed 4 years ago

Dahianna commented 5 years ago

Now that we changed the design for the blog template we need to update the blog post template in the guide.

Dahianna commented 5 years ago

So, I looked back at the original issue in the 18F-site repo and it was created by someone who left 18F. I checked the guide and it seems to be up to date to me.

Dahianna commented 5 years ago
cmajel commented 5 years ago

Per chat, we could try using this include that works on other parts of the guide, and see if it fills in the code accordion.

{% include details-code.html title='testimonial' content=testimonial_codeblock lang="html" description=testimonial_description %}
Dahianna commented 5 years ago

I tried to fixed it for the sidebar but it didn't work. Not sure if I have any mistakes on the code https://github.com/18F/18f.gsa.gov/pull/3078

cmajel commented 5 years ago

I can look at this and see if I can figure out why this fix isn't working. Otherwise, we may need a dev consult.

cmajel commented 5 years ago

Alright, I looked into the missing code blocks and I have a theory. Putting my notes here while I continue to investigate.

*18f site repo https://github.com/18F/18f.gsa.gov

It appears that the code block do have content inside, but sometimes the div wrapping the codeblock's content is missing a aria-hidden="false" flag that actually allows it to be displayed.

Sidebar example, where there is no flag

codeblock-no-flag--sidebar

Breadcrumb example, where is there is a flag

codeblock-with-flag--breadcrumb

When I manually add the flag to the sidebar (and other code components where the code block doesn't open), it appears.

I'm not sure why the flag appears on some blocks and not others. But, I can take another look next week. Or, @Dahianna if there is a dev helping with the 18F site, they may be able to give a faster answer.

cmajel commented 4 years ago

Adding a note that I believe the base code for this accordion component comes from details-code.html and that the div is populated here in lines 22-24. However, I don't see an obvious way for a aria-hidden="false" flag to be applied to the div on line 22.

Screen Shot 2019-12-03 at 5 15 34 PM

I also confirmed that when I hardcode a aria-hidden="false" on that line 22 div, it opens the accordion by default, and doesn't toggle on touch.

adunkman commented 4 years ago

Looks like there’s an ID collision happening preventing the accordion from functioning properly.

image

The aria-hidden flags are added by JavaScript, and the JavaScript is finding h3#sidebar rather than div#sidebar as intended.

This is a collision between auto-generated IDs for the heading and auto-generated IDs for the accordion. A quick, easy, and perhaps right fix would be to change details-code.html to add a suffix to its IDs so it generates something different — perhaps {{ include.title | slugify }}-accordion?

cmajel commented 4 years ago

@cmajel to take a look and see if this branch is possibly out of date and causing the issue. If not, ping eddie again on the build error.