hugo-universal-theme icon indicating copy to clipboard operation
hugo-universal-theme copied to clipboard

Accordion implementation

Open ringods opened this issue 7 years ago • 10 comments

Initial implementation of an accordion shortcode. This PR is work in progress but is already created to start a discussion.

The Hugo docs on shortcodes mention this:

A shortcode is a simple snippet inside a content file that Hugo will render using a predefined template. Note that shortcodes will not work in template files. If you need the type of drop-in functionality that shortcodes provide but in a template, you most likely want a partial template instead.

This morning, I finished this initial implementation, but I'm actually not satisfied with it. Thinking a bit more about the different shortcodes mentioned in the upstream template, converting them to shortcodes in this Hugo template is the most obvious way of implementing this.

Why am I not satisfied? A shortcode for youtube or vimeo is still focussed on including content. The accordion and accordion-item shortcodes I created are mainly a layout matter. The other upstream shortcodes are also about layout. Hugo pushes everything related to layout to a theme or custom layouts, so that's why I think a Hugo shortcode doesn't make sense.

I will still give this a second thought, but if someone else has ideas on how to solve this with templates, partials or other layout related Hugo features, please chime in.

While I created these shortcodes, I ask you for the moment to not merge this!

Part of #101

ringods avatar Aug 03 '17 07:08 ringods

sounds great to me @ringods

GeorgeWL avatar Aug 04 '17 13:08 GeorgeWL

@adrianmo @GeorgeWL I have taken some more time to come up with a better split between content and layout. Please have a look at commit 7c7e698d363e3d1887a0fda450f1c9d600e392df I added to this PR.

accordion and accordion-item are not partials rather than shortcodes. To use these, one should add a shortcode to fetch the data and use that in the content files. The exampleSite contains a shortcode reading from .Site.Data and passing the maps to the main accordion partial. This one iterates over each of the files and calls the accordion-item partial.

This PR still lacks setting some div attributes to handle the accordion collapsing, but I'll add that once the overall setup is considered better than my first attempt.

Please review.

ringods avatar Aug 23 '17 07:08 ringods

@adrianmo @GeorgeWL @ryanfox1985 I completed the accordion setup and I am quite satisfied with the outcome. I replaced the clumsy Scratch usage by dict.

Please let's review this all and get this merged. I have more in my fingers to contribute. :-D

ringods avatar Aug 25 '17 07:08 ringods

@adrianmo @devcows

GeorgeWL avatar Aug 31 '17 20:08 GeorgeWL

ping @adrianmo

ringods avatar Sep 09 '17 16:09 ringods

Does this look good to you @adrianmo ?

dabibbit avatar Dec 09 '17 00:12 dabibbit

@ryanfox1985 @adrianmo when could this be merged?

ringods avatar Feb 09 '18 09:02 ringods

@ryanfox1985 @adrianmo ping.

ringods avatar Mar 03 '18 20:03 ringods

@ringods I've just pulled in your PR and tried out your accordion implementation. It's awesome, thanks a lot! :bowing_man:

What I would change from a beginner's perspective:

  • Rather name the shortcode accordion instead of accordion-example. You can still explain in the README how "advanced users" are supposed to adapt this shortcode etc., but it would be more convenient with the shorter name to just use your example shortcode :slightly_smiling_face:
  • Improve the README section about the accordion: Basically provide two subsections: The first one to simply explain how to use the already existing shortcode and adapt the .yaml files under data/accordion-data. The second section about the accordion partial etc. (this is already there).

BTW: This should absolutely be merged!!!

salim-b avatar Sep 16 '18 22:09 salim-b

I am currently trying to hack away at this myself and think this should be added!

yeelauren avatar Sep 27 '20 20:09 yeelauren