Skip to content
This repository has been archived by the owner on May 5, 2021. It is now read-only.

Add grouping & accordion functionality to the menu list #165

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

jaredshaunsmith
Copy link

My team needed lots of components, and pages - it became very unmanageable with the menu so long. I added an accordion functionality to Fabricator to solve this.
screen shot 2015-08-18 at 1 11 21 pm
screen shot 2015-08-18 at 1 11 08 pm

@jaredshaunsmith
Copy link
Author

@LukeAskew happy to hear any feedback. think this is a useful addition to fabricator for sure. we love it and have been using it to build an internal pattern library for quick starting client projects. thanks for putting it together!

@qaid
Copy link
Contributor

qaid commented Aug 19, 2015

@jaredshaunsmith - nice! 👍

@jaredshaunsmith
Copy link
Author

added a condition under the setActive() function to check if window hash matches with an accordion. previous commits had the accordion remaining closed when a user refreshed, or loaded straight to a hash. now the accordion will open when the page is loaded to a hash.

@LukeAskew
Copy link
Member

This is awesome. Thanks for putting this together. A few feedback items before this gets pulled.

  1. Can you clean up the PR a bit? There are some false positive commits where you removed an empty EOF line and some where files were renamed.
  2. I think the icon should also be a toggle. Clicking on the icon should open/close the accordion without navigating to the section. This would give a better scan/search experience.
  3. The accordion item's href hash should have {{@key}} not {{@../key}} so that a click links to the item and not its parent.

What does everyone think of "+" and "-" instead of "▼" ? I think the +/- is a little lighter and cleaner.

Let me know your thoughts. If we agree with the icon click toggle and/or the +/- idea I can help add an SVG symbol to the icon set.

@bbodine1
Copy link

I agree. A + and - makes more sense.

@qaid
Copy link
Contributor

qaid commented Aug 24, 2015

I also agree with the "+" and "-" instead of the arrow. I implemented @jaredshaunsmith's tweak and the sidebar does get a bit crowded with the arrow character. Otherwise I really like have the accordion effect and grouping.

@jaredshaunsmith
Copy link
Author

@LukeAskew
Sure thing! Some responses to your feedback below.

  1. Totally, will clean up.
  2. I agree from a UX perspective. Good call. Technically will require a refactor though, since these icons are :after elements right now, and I'm pretty sure we can't target a click on those independent of their parent element... correct? Shadow dom and all that.
  3. Yep, good call.

Updated to "+" / "-" - will push in a bit to update the PR.

Thanks.

@jaredshaunsmith
Copy link
Author

@LukeAskew
Ok!

  • Cleaned up PR a bit, left the file name changes, as those were just a further example of how to group items into an accordion.
  • Refactored icons to be in their own element and added javascript to control those individually from actually going to the href.

@LukeAskew LukeAskew mentioned this pull request Oct 8, 2015
@DJTB
Copy link

DJTB commented Jan 5, 2016

Just a +1... I'd love to see this merged into master if the conflicts are easily resolved.

@muckinger
Copy link

Also a +1 for merging this feature

@jonorherrington
Copy link
Contributor

@LukeAskew where are we on this one? I also think this would be a great one to push.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants