[WDS-1193]: Fixes javascript issues in styleguide#1198
Conversation
|
@el-mapache thanks! I just checked and the code sample boxes are there, but not opening on click.
|
|
The homepage uses some polyfill scripts in We can either create a scripts-homepage.js with just those, or add them manually to |
| toggleFormInput($('.usa-show_password'), 'Show Password', 'Hide Password'); | ||
| toggleFormInput($('.usa-show_multipassword'), 'Show my typing', 'Hide my typing'); | ||
| validator($('.js-validate_password')); | ||
| if (/form-controls/.test(window.location.pathname)) { |
There was a problem hiding this comment.
Just realized something about this:
The code compiled from start.js is distributed as part of the WDS separate from the docs site, but this code is assuming you're on the docs site because it's testing for URLs of the docs site. So if a user had the standards on their own site, this if statement wouldn't be called correctly because they won't have a form-controls page (it's really confusing knowing how the WDS is separate from the docs site)
What is this check attempting to protect from? Would it make sense to uses classes rather then URL's to test for existence?
There was a problem hiding this comment.
I agree @msecret good catch. This should be targeting classes or ids related to form-controls.
There was a problem hiding this comment.
The check is to prevent the code from instantiating those plugins if the DOM elements aren't on the page. We could definitely just check individually for the existence of those elements.
I used URLs because it was faster than checking for each element individually, and because I didnt consider that other users might be installing the docs :(
There was a problem hiding this comment.
Well the src/js/start.js isn't a part of the docs/ but rather a part of the package. It can be very confusing. Sorry @el-mapache.
There was a problem hiding this comment.
Ah, so, in that case, we should probably move docs specific code to the styleguide.js file, correct? Initializing the all the plugins with their respective DOM elements wouldn't relevant to the overall package, just to the docs pages.
This patch moves Ethnio to analytics.html as it should be on every page and not be tied to code specific to the styleguide.
|
I added a small commit to move Ethnio into |
|
@maya Is it crazy to unify the start.js and styleguide.js files? The code samples accordion isn't opening because that content is dynamically added in the styleguide.js file. But, the accordion plugins are initialized in start.js (which we need to load first to make jquery available). If unifying these files isn't possible we could put our accordion plugin under an 18F namespace on the window object. Then, we could initialize the code samples section as an accordion once its been added to the page. |
|
@el-mapache It wouldn't be a good idea as the |
|
@rogeruiz That makes sense. So, we should still be able to move the docs page specific plugin initialization from start.js and put it into styleguide.js? |
|
@el-mapache absolutely. that would be good to have in this PR to help avoid this in the future. The |
|
@el-mapache This commit seems to have reverted all the changes rather than just some of them. Only the jQuery |
|
Also, the styleguide site and homepage have different JS requirements @rogeruiz @el-mapache - i.e. the homepage doesn't use JS, while the website does. |
Fixes javascript errors in styleguide as described in issue 1193: #1193
Use the title line as the title of your pull request, then delete these lines.
Description