Suggestion on subtab syntax

[tovodeverett]

I generally like the Tabulous 2 syntax, but I find the subtab syntax a little clunky. The positional nature of tab declarations make sense, but if I'm quickly scanning through something with a lot of tabs and subtabs, it's easy to loose sight of which are which. I've taken to adding a gratuitous indent to my subtab declarations, but I wonder if it might make more sense to use an approach like so:

tabs do
  unicorns_tab do
    ...
  end

  rainbows_tab do
    ...
    active_when { a_subtab_is_active }

    monorainbows_subtab do
      ...
    end

    double_rainbows_subtab do
      ...
    end
  end

  ponies_tab do
    ...
    active_when { a_subtab_is_active }

    shetland_ponies_subtab do
      ...
    end
  end
end

The advantage of this syntax is that it's easy to leave it backward compatible - just leave the existing syntax and permit *_tab blocks to stash *_subtab calls and then execute them after the do block for the *_tab finishes. That said, there may be perfectly good reasons not to support such a syntax.

Thoughts?

[techiferous]

Hmmm...that sounds like a great idea to me.

While we're talking about syntax, I had an idea that it's not necessary to name the tabs and subtabs. For example:

 tabs do
    pictures_tab do
      text          { 'My Pictures' }
      link_path     { root_path }
      visible_when  { true }
      enabled_when  { true }
      active_when   { in_action('any').of_controller('pictures') }
    end

    music_tab do
      text          { 'My Music' }
      link_path     { songs_path }
      visible_when  { true }
      enabled_when  { current_user.has_music? }
      active_when   { a_subtab_is_active }
    end
end

Becomes:

 tabs do
    tab do
      text          { 'My Pictures' }
      link_path     { root_path }
      visible_when  { true }
      enabled_when  { true }
      active_when   { in_action('any').of_controller('pictures') }
    end

    tab do
      text          { 'My Music' }
      link_path     { songs_path }
      visible_when  { true }
      enabled_when  { current_user.has_music? }
      active_when   { a_subtab_is_active }
    end
end

What do you think? Do you find value in naming the tabs?

[tovodeverett]

The only value in naming the tabs would be if there were an interface that allowed one to somehow manipulate the tab sets at run time. Also, I do find the music_tab syntax a little strange - method_missing interfaces can be clever, but I think the tabs(:tab_set_name) approach you use for naming tab sets is easier to read and understand.

[tovodeverett]

One idea I had is to manifest the tab/subtab/tabset names as id attributes in the generated HTML so that styling can be applied to individual elements using CSS (and the elements can be easily accessed via JavaScript). My suggestion would be to make the names optional, to allow them to be passed as a parameter (similar to how tabs and subtabs handle names), and to only expose them as id attributes and through APIs if they are explicitly set. The existing syntax (music_tab) should be retained for compatibility, but you might consider deprecating it.

[techiferous]

These are some great ideas! Thank you!

[tovodeverett]

Another reason for implementing id attributes based on the tab/subtab/tabset names would be to make acceptance tests more robust.

[techiferous]

Yes, very good point! Thanks!

[techiferous]

@tovodeverett I almost included adding id attributes in my latest release of tabulous (2.1.0) because I liked the idea so much, but I hesitated because it has a non-zero cost (adds some bloat to the markup).

So before adding it, do you have a specific use case in mind that needs it? I understand the argument for acceptance testing, but I can also see an easy way to acceptance test this with Capybara, which understands link text scoped within an HTML element.

I also understand making it easier to target a single tab for specific CSS or JS behavior, but does this happen often?

Also, I'm hoping some others will chime in on the other API suggestions you made here. I try to go slow on API changes to be friendly to the users, since I already forced them to do some work when I broke backwards compatibility with tabulous 2. :)

[tovodeverett]

While one could use the link text for acceptance testing with Capybara, that does somewhat complicate things if one is using I18n for the tab text. If I have something that reads, click_link :artist_tab, it's pretty obvious in the test what the test is trying to do and it's likely to be reasonably robust. The alternative is something that either focuses on where the link goes (which is more vulnerable to link duplication and embeds knowledge of the destination in the link identification logic) or one that uses the tab text (which embeds knowledge of the tab text, which really should be part of the view aesthetics rather than structural).

I won't fight too hard for the feature since it's easy enough to monkey patch it in (that's what I'm doing currently). That siad, if you want to have your cake and eat it to, consider doing it as part of eliminating the requirement that tabs be named. If the tabs don't have to be named, then you could emit the id attribute only when the user explicitly names the tags and skip it when the tabs are unnamed.

[techiferous]

Great point about i18n.

If we do put in id attributes, I would just keep the current requirement that tabs be named. That seems simpler, and I want to keep tabulous very simple.

Since this doesn't seem like a change that needs to be made quickly, I'll just leave this issue open and hope that others chime in as well. We can see how much of a pain point this is for other users, too.

[barendt]

For whatever it's worth, I've got a need to target specific tabs with JavaScript, so it'd be handy to be able to include an id attribute with tabs.

I'd be happy to come up with something and submit a PR if that's something to which you'd be receptive.

[techiferous]

Hi @barendt!

First of all, would you mind going into a little more detail on why you would need to target specific tabs with JavaScript? I'd just like to get to know better the different use cases people have with tabs so that I can better determine which are the common ones.

And secondly, I think tabulous already supports giving an id attribute to tabs; it just doesn't do it automatically. You shouldn't need to fork tabulous; you should be able to create your own custom renderer. Does that make sense or am I misunderstanding your need?

[barendt]

Our use case is probably a little unusual - we're reusing a controller action in a couple of different contexts. Everything about the action is the same, but if the user has gotten to that action via the administrative interface we want the admin tab to be active instead of the tab that would normally be active.

I think you're right that we could do this with a custom renderer; that hadn't occurred to me, but it makes sense. Right now we're just using a little bit of JavaScript that'll make sure the active class is on the tab we want based on a query string parameter. It's not awesome, but it works.

[techiferous]

Thanks for the details! Yes, that's an unusual use case for tabulous (since it has a design assumption that the active tab is based on controller actions) but using JavaScript is a pretty clever way around it. I hope the custom renderer gives you what you need.