projectblacklight / blacklight

Blacklight provides a discovery interface for any Solr (http://lucene.apache.org/solr) index.
http://projectblacklight.org/
Other
757 stars 257 forks source link

View component tools support #3106

Closed body-clock closed 8 months ago

body-clock commented 11 months ago

Addresses #3102

ViewComponent 3.0.0 switched the Slots API and how they're called. This PR addresses some code that had not been updated to reflect those changes. It also aims to test that the Tools components render their modals properly to prevent similar issues in the future.

Just one note - because the citation tool relies on methods not present in core Blacklight, the tool's functionality cannot test that the modal content renders correctly. I've chosen to just test that the header renders correctly. Through my investigation, I've also noticed that the Blacklight Demo doesn't have a working citation tool - something to think about.

jcoyne commented 9 months ago

Will this still work with view_component 2? The gemspec indicates that 2 is an acceptable version.

body-clock commented 9 months ago

Will this still work with view_component 2? The gemspec indicates that 2 is an acceptable version.

I'm having a hard time figuring this out. The ViewComponent changelog for 3.0 lists this as a breaking change, but it seems this convention has been adopted throughout the rest of the Blacklight gem. For example app/views/catalog/facet.html.erb uses component.with_prefix, component.with_title, and component.with_footer, following the conventions of VC 3. We're using VC 2.66 in our project and I'm not seeing any major issues with components that have adopted this.

jrochkind commented 8 months ago

Will this still work with view_component 2? The gemspec indicates that 2 is an acceptable version.

@jcoyne , the gemspec requires at least view_component 2.66, not actually any 2. https://github.com/projectblacklight/blacklight/blob/8eb4d3a0c388183ca0f24f160b86f0ca4ec84e2a/blacklight.gemspec#L34

The "newer" with_ API is available from 2.54.0 on. https://viewcomponent.org/CHANGELOG.html#2540

So should be good!

jcoyne commented 8 months ago

Thank you for taking the lead on this @body-clock. I think I've incorporated all this into #3120 and merged into main.