rvanhorn / hugo-dynamic-tabs

A Hugo theme component that allows you to add dynamic tabs in your markdown files.
https://hugo-dynamic-tabs.netlify.app/
MIT License
53 stars 14 forks source link

Improving the markup of tabbed panes #8

Closed deining closed 2 years ago

deining commented 3 years ago

Meanwhile I evaluated this project, and to be honest, my impressions are mixed:

Pro:

Concerns:

The choosen implementation looks a bit complicated to me:

All these deficiencies make your solution error-prone. Your fact that you are authoring a section Common Issues somehow indicates that you are already aware of this.

My tipp: have a look at the implementation proposed here. This solution looks much cleaner to me.

Further note: For me the best markup would look like:

{{< tabs >}}
{{< tab header="Tab 1" >}}

**Tab 1 Content**

{{< /tab >}}
{{< tab  header="Tab 2" >}}

**Tab 2 Content**

{{< /tab >}}
{{< tab header="Tab 3" >}}

**Tab 3 Content**

{{< /tab >}}
{{< /tabs >}}

This would allow rearranging the order of the tabs without the need for any subsequent corrections. Maybe you want to rework your current implementation following up on my proposals/comments? This would make your project even more useful and better IMHO.

One final thought: I would propose to add a README.md inside the demo directory, explaining how to run the demo locally once the directory was cloned.

Again, thanks for your project, I really appreciate your work.

rvanhorn commented 3 years ago

I originally created this shortcode as my old documentation site used tabs, however shortly after I refactored the documentation site and got rid of the tabs. This is primarily why I haven't really updated this since 2019. Your improvements are pretty much in-line with my own assessment and I did do some work towards them.

parameter tabTotal seems redundant to me: the program logic can count the number of inner tab shortcodes and will end up with the same number

Already changed so its not longer needed.

tabName1="Tab 1" tabName2="Tab 2" tabName3="Tab 3": your implementation uses named parameters, this means you have to alter all existing tabNameX if you insert a tabName1 at the very beginning. This is inconvenient and could be avoided by using unnamed parameters

The original issue I ran into was linking the individual tab to the parent, I'll look into this.

parameter tabNum seems redundant to me: we can simply loop over nested tab shortcodes in the order given and increment the number one by one. Currently, if one wants to insert a tab at the very beginning, he has to adjust tabNumfor all existing tabs, which is kind of a hassle

Already changed so its not longer needed.

Unique tabID: ideally a random unique ID were generated by the program logic, freeing the user from giving this parameter.

Already changed so its not longer needed.

I'll see what I can do. :)

rvanhorn commented 3 years ago

Alright so I lied @deining, I started working on it shortly after my post. :)

As you can see in 8ffe780867afc0d3dc394cda428a058a60a15d43, there are only two variables now, the tabTotal in the Tabs Shortcode and tabName in the Tab Shortcode which solves all but one of the issues you've addressed. I wasn't able to remove tabTotal and that's because from what I've read, Hugo shortcodes render the inner shortcode first. I looked at the example you provided and see how they do it but its a lot more work than simply inputting the number of tabs.

There are a few bugs left to fix, like multiple tabs on the same page which I'll probably work on later this week.

deining commented 3 years ago

I appreciate your work on this.

from what I've read, Hugo shortcodes render the inner shortcode first.

That's very true. Then you have to find other ways :-)

See my answer on Stackoverflow: it showcases how to overcome the problem by creating a data structure, containing the data of all tabs and passing that structure to the parent's scratch. The parent has then all data at hand and may render whatever is needed. As mentioned on stackoverflow, I contributed to the docsy theme, allowing tabs to be used inside this theme. Maybe you can get some inspiration from this. Feedback is welcome, I see several areas, where my solution could be extended and improved.

Keep up your work, I'm looking forward to new version 2.0!

rvanhorn commented 3 years ago

Your solution is pretty great!

I didn't know it was possible to store the tabs and their content into a data structure, that's pretty neat. Unfortunately for me, even if I were to expand on it, I would have to copy/paste your solution to implement anything similar.

deining commented 3 years ago

Don't try to reinvent the wheel! Feel free to take over my solution and build upon it and adapt it to your needs. Good luck with your coding!

rvanhorn commented 2 years ago

Released V2.00 with my solution, maybe one day I'll revisit and come up with something like you did. :)