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

Add output_preamble to match existing output_postamble #1931

Open
kdonovan opened this issue Dec 12, 2023 · 5 comments
Open

Add output_preamble to match existing output_postamble #1931

kdonovan opened this issue Dec 12, 2023 · 5 comments

Comments

@kdonovan
Copy link

ViewComponent::Base defines an output_postamble for additional content that will be appended to the rendered string, but currently there is no matching hook for prepended content.

Motivation

First is just providing a consistent interface/not falling afoul of the principle of least surprise. But beyond that, I have a use-case where I'd like to wrap every view component in a specific HTML tag (with data-controller and class attributes set based on the filename, so sidecar stimulus controllers/css modules from view_component-contrib can be applied automatically). With the current state of the codebase I can add the closing tag, but there's no way that I can see to inject the opening tag.

It looks like a fairly minor change, and I'd be happy to submit a PR if there's interest, but I wanted to see if there's any context I'm missing first.

@camertron
Copy link
Contributor

Hey @kdonovan, this seems like a good idea. How do you feel about submitting a PR?

@kdonovan
Copy link
Author

I went ahead and added a small PR for this. CI is failing... the changes are simple and it looks like it's failing for everyone else, too, but tbh I never got the bundle exec appraisal install step to work so I haven't been able to verify the tests still pass locally. 😬

Screenshot 2023-12-13 at 12 56 13 PM

@camertron
Copy link
Contributor

It looks like these test failures are legitimate. I left a comment on the PR.

As far as running tests locally, I would recommend getting bundle and bundle exec rake test working first, then trying to use Appraisal.

@reeganviljoen
Copy link
Collaborator

@kdonovan I recently started adding a few things, and also had to get setup, if you need help you can ping me in the PR

@kdonovan
Copy link
Author

Thanks folks -- just using bundle + bundle exec rake test got me unblocked! I'm still not quite understanding why the PR was getting FrozenErrors trying to concat the string (maybe something to do with the compilation step?), but I just pushed up a different approach (joining an array) that at least passes tests locally.

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

No branches or pull requests

3 participants