-
Notifications
You must be signed in to change notification settings - Fork 110
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
Bootstrapped Accordions on Write Plan page #629
Conversation
- Bootstrapped the Write Plan page accordions - Rebased with latest from CDL-MVP bootstrapped the phases accordion DMPRoadmap#567 added es6 function to provide expand/collapse all functionality. bootstrapped several accordions DMPRoadmap#567 finished Bootstrapping write plan accordions DMPRoadmap#567 fixed linter issue DMPRoadmap/roadmap#issue567 fixed linter issue
@@ -0,0 +1,47 @@ | |||
import 'bootstrap-sass/assets/javascripts/bootstrap.min'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only import the bootstrap plugin that you are going to use (e.g. ... javascripts/bootstrap/collapse.js
* </div> | ||
* </div> | ||
*/ | ||
export default () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice collapsible abstraction!
|
||
$(() => { | ||
// Attach handlers for the expand/collapse all accordions | ||
expandCollapseAll(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the module expandCollapseAll always walks through any accordion-controls class, i.e. the selector is hardcoded in the module, perhaps is better if we rename this filename to something more generic since it will be used in multiple views, not only at phases/edit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good! The minor changes can be addressed in further PRs.
Profile" page. Change- Added can_use_api? to the User model method can_org_admin. (Noticed, that if only can_use_api? is set, then the user sees the Admin Plans and Usage.)
other Org Admin privileges not set." Changes: - Replaced @user.can_org_admin? (in tests for display of API Access tab and content) with @user.can_use_api?. This insures that tabs visible in case it was failing.
…hts_not_working IssueID #629 - Fix to allow API Access tab visible on the users's "Edit
Bootstrapped the accordions on Write Plan page #567
Note that the expand/collapse all links on the section accordion expand/collapse all of the guidances contained within the sections as well (the guidance is a nested accordion within the section accordion panel). Don't see a way around this without hacking Bootstrap a bit.
Will include tests for expandCollapseAll.js on Monday (forgot to do a
git add
from my work machine yesterday)