Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

View component tools support #3106

Conversation

body-clock
Copy link
Member

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.

@body-clock body-clock marked this pull request as ready for review November 9, 2023 22:20
@jcoyne
Copy link
Member

jcoyne commented Dec 27, 2023

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

spec/features/tools_spec.rb Outdated Show resolved Hide resolved
@body-clock
Copy link
Member Author

body-clock commented Jan 4, 2024

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
Copy link
Member

jrochkind commented Jan 17, 2024

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.

s.add_dependency "view_component", '>= 2.66', '< 4'

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

So should be good!

@jcoyne
Copy link
Member

jcoyne commented Jan 18, 2024

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

@jcoyne jcoyne closed this Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants