no list accordions#1137
Conversation
Fixes #1136 I am not sure what the conversation around placing each accordion within a list item was; however, it feels less than semantic to have a list item containing a button and a div…imagine a text document with a list item containing an entire chapter of a book, for example. I’m also curious if a more suitable tag (say, section) would be more proper, given the specification of a div and section: http://www.w3.org/TR/html5/grouping-content.html#the-div-element - The div element has no special meaning at all. … Authors are strongly encouraged to view the div element as an element of last resort, for when no other element is suitable. http://www.w3.org/TR/html5/sections.html#the-section-element - The section element represents a generic section of a document or application. Changing to a section tag could get us toward a no JavaScript solution to the accordions (#1122 and #1121) - basically, add the buttons and collapse the content if JS is present. Could result in “flashing” content though prior to JS kicking in.
|
Hi @joshbruce, we'll have to take a deeper look into the semantics of the accordion. Thanks for bringing it up. The team is currently at an offsite, so apologies for the delay. cc @nickbristow |
|
@joshbruce, I commented on #1136, I'm not sure I agree with removing the list aspect from the accordions. |
| @@ -11,21 +11,6 @@ $accordion-border: 3px solid $color-gray-lightest; | |||
| margin-top: 1rem; | |||
There was a problem hiding this comment.
So unfortunately the files in docs/ are not actually the source files and shouldn't be hand edited. The files in src are where the scss files should be edited which will then be copied into docs.
I'm sorry for this confusion, it was done to limit complications with the Jekyll docs site and we're actually working on fixing it right now. In the meantime, it would help us if you could move your code changes in this file to src/stylesheets/components/_accordions.scss and then run the build process with npm run build or the more specific gulp tasks.
Let me know if you'd like some help with this. You've been a great contributor so I'd be happy to get these files into the correct form.
There was a problem hiding this comment.
Thanks @msecret!
I thought that's where we were going with gulp.
Would the gulpfile.js found here help inform the fix you are looking to make with the gulp build? (https://github.com/BrianGilmanGSA/sam-web-design-standards)
Right now I cannot run a gulp build (see #1148) - I can only do $ ./go serve to confirm the changes, which doesn't update the /docs. If you could help me out on that score, I would be more than happy to make changes and resubmit along with using the /src directory for future PRs.
Thanks again.
There was a problem hiding this comment.
Sorry, meant the website.js file: https://github.com/BrianGilmanGSA/sam-web-design-standards/blob/gh-pages/config/gulp/website.js
Note: There are changes to the other /config/gulp files as well to separate out the /src from the /scss for the Jekyll build.
There was a problem hiding this comment.
Sure let's work through it.
What happens when you run gulp website:serve? Is there an error you could show me?
There was a problem hiding this comment.
I'm going to clone directly from the 18f repo instead of my fork - just to make sure.
- Cloned 18f/web-desing-standards
$ cd ~/path/to/repo- Completed
- Ran
bundle update - Completed
- Ran
npm update - Completed
- Ran
$ gulp website:serve - Halted with:
jekyll 3.1.3 | Error: File to import not found or unreadable: lib/bourbon. Load path: /path/to/repo/docs/_scss on line 3
It appears /src/stylesheets/lib no longer exists: https://github.com/18F/web-design-standards/tree/18f-pages-staging/src/stylesheets
On the initial clone, /docs/_scss/lib does exist: https://github.com/18F/web-design-standards/tree/18f-pages-staging/docs/_scss
When $ gulp website:serve removes /docs/_scss the /docs/_scss/lib directory is removed and not replaced.
Let me know if you experience something different - or, of course, if I'm doing this incorrectly.
There was a problem hiding this comment.
Fascinating.
Still needed the $ bundle update - just in case that shouldn't be necessary.
Can you point me to why install versus update makes the difference?
Reading the Terminal didn't seem much different.
There was a problem hiding this comment.
@joshbruce first off, did that solution work for you at all?
Based on the documentation on npm, and what I've read, there doesn't seem to be a large difference(http://stackoverflow.com/questions/12478679/npm-install-vs-update-whats-the-difference), I gave you that command just to ensure we're both doing the same thing to make debugging a little easier.
There was a problem hiding this comment.
@msecret - Sorry, should have been more explicit.
Yes. The build completed.
I'm not sure what difference there was, but it was enough for /src/stylesheets/lib To appear. Not sure why though, as I thought install and update only effected the /node_modules directory.
There was a problem hiding this comment.
@msecret - Just a quick follow up - I'm a curious soul - sorry for blowing up your inbox. Wanted to show some screenshots to isolate when it happens.
Before $ npm install
After $ npm install
The point at which the /src/stylesheets/lib directory appeared.
Looks like $ npm update does not run the $ gulp copy-vendor-sass - from the npm prestart (??), which makes sense. To test that hypothesis - I did the following.
$ bundle update- to avoid the kramdown error (must be done prior to$ npm start).$ npm update$ npm start
It worked. Which makes sense as calling $ gulp website:serve does not appear to call $ gulp copy-vendor-sass. If the goal is to not call gulp directly - then it works as expected.
There was a problem hiding this comment.
Ah @joshbruce I figured it out:
In package.json we have a prepublish script that executes certain commands after npm install (which doesn't make much sense). I guess those have to be run before website:serve.
I'll keep this in mind when working on improving the dev workflow this week.
|
@joshbruce I created #1153 to restore the functionality of Thanks, and of course kit with any comments/questions, I hope I've thought of everything here :) |
Fixes #1136
I am not sure what the conversation around placing each accordion
within a list item was; however, it feels less than semantic to have a
list item containing a button and a div…imagine a text document with a
list item containing an entire chapter of a book, for example.
I’m also curious if a more suitable tag (say, section) would be more
proper, given the specification of a div and section:
http://www.w3.org/TR/html5/grouping-content.html#the-div-element - The
div element has no special meaning at all. … Authors are strongly
encouraged to view the div element as an element of last resort, for
when no other element is suitable.
http://www.w3.org/TR/html5/sections.html#the-section-element - The
section element represents a generic section of a document or
application.
Changing to a section tag could get us toward a no JavaScript solution
to the accordions (#1122 and #1121) - basically, add the buttons and
collapse the content if JS is present.
Could result in “flashing” content though prior to JS kicking in.