Switching tabs doesn't work

[dkniffin]

Switching tabs doesn't work

[allmarkedup]

@dkniffin I'm afraid I haven't been able to replicate this at all ☹️ Lookbook should be isolated from your app's JS so I'm not sure what might be going on here.

Because I can't replicate it and the demo seems to be working for you I think I'm going to close this down for now - if you see the issue repeated elsewhere or manage to get any other clues about what could be causing it please feel free to reopen and I'll try to look into it again.

I'm sorry this is getting in the way of you using Lookbook and hope you find a solution 😕

[allmarkedup]

Hey @dkniffin, sorry to hear you are running into a problem here and apologies for the slow reply. You shouldn't need to include any JS or configure anything, this should just all work out of the box.

Could you let me know what browser you are using so I can look into it a bit more when I have a chance?

And do you think you could just confirm whether or not the tabs on the demo site (http://demo.lookbook.build/lookbook/inspect/elements/avatar/default) are working for you or not?

[dkniffin]

Brave v1.50.114, Chromium: 112.0.5615.49
on MacOS 12.5.1

And yes, the tabs on the demo site are working. So likely something specific to my app or configuration

[dkniffin]

@allmarkedup I'm starting to dig into this a bit more. Comparing the attributes on my app vs the demo, I see @click.prevent isn't set on my tabs. From what I can tell, that gets set here. Any idea why it wouldn't be set for me?

[dkniffin]

Dug in some more. I forked the repo and pointed to it locally, and put some debuggers in. This is happening for me with all buttons. I'm testing with the refresh-preview-button, which includes @click.stop.

When execution gets to this line, html_attrs is correct and includes @click.stop.

From there, it goes to here and html_attrs still has @click.stop.

But when it gets to here, @html_attrs no longer has @click.stop

[dkniffin]

I found the issue. It's something custom to my app. We've overridden our ViewComponent::Base to set an attribute for debugging, that will tell us which component is outputting each part of the DOM:

module ViewComponentDebugOutput
  def render_in(*args)
    value = super

    return value unless Rails.env.development? && ENV["VIEW_COMPONENT_DEBUG_ATTRIBUTES"] == "true"

    begin
      fragment = Nokogiri::HTML.fragment(value)
      # We can't do anything useful if the ViewComponent doesn't have a single root element
      return value if fragment.children.length != 1
      root = fragment.children[0]
      root["data-vc-debug-name"] = self.class.name
      value.html_safe? ? root.to_html.html_safe : root.to_html
    rescue => e
      value
    end
  end
end

ActiveSupport.on_load(:view_component) do
  ViewComponent::Base.prepend(ViewComponentDebugOutput)
end

Something about this causes the issue. When I comment all that out, the issue is solved.

I'll figure out what to do from here

[allmarkedup]

Hey @dkniffin, thanks for digging into this and apologies for not replying sooner.

So based on your info above, I believe what is happening here is that Nokogiri is not liking the non-standard HTML attributes names that AlpineJS uses (at least in shorthand format) and is stripping those out when re-assembling the DOM in your render_in method above.

And example of one of these attributes is the @click attribute on the tab component:

lookbook/app/components/lookbook/tabs/tab/component.html.erb

Line 18 in bd1d3be

"@click.prevent": "selectTab($el)",

- which would explain why the tabs are not working for you.

This is obviously a bit of a niche issue but as it may still affect others I'm going to swap the shorthand Alpine HTML attributes for their more 'standard' long form versions - I suspect that Nokogiri will tolerate those a bit better and there is now downside really (apart from a little more verbosity).

I'm going to reopen this issue now and will hopefully get a fix out in the next release unless I run into any unforseen issues.

[allmarkedup]

@dkniffin I've just opened a PR (#467) that removes replaces the shorthand attribute names. If you have a spare few minutes could you try pulling in Lookbook from that branch and seeing if that fixes the issue for you, even with your debug fragment in place? Thank you!

[dkniffin]

@allmarkedup Yes, I can test it. But FYI, I have since deleted that debugging fragment in favor of the built-in annotate_rendered_view_with_filenames Rails config.

Edit: Confirmed, that PR fixed it ✅

[allmarkedup]

@dkniffin thanks for the confirmation! Much appreciated. The fix has gone out in the latest 2.0.3 release so I'm going to close this down now.