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 support for append and prepend on collection_select (fixes #428) #641

Merged

Conversation

judylovesruby
Copy link
Contributor

Hello,
thanks a lot for creating this gem!
Today I would have needed a collection_select with prepend and noticed that this does not work yet.
Here's a fix for it. I hope everything's alright and would be happy if it got merged 🙂
Best,
Judith

Copy link
Contributor

@lcreid lcreid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR. It's very much appreciated. I haven't yet had time to dig into it -- the PR looks good but it raises some questions in my mind about other methods that don't support prepend and append. If I recall correctly, Bootstrap 4 had specific classes that didn't apply to all types of form elements, but I could be wrong about that.

Do you think there are other methods that don't support prepend and append, but could and should?

@judylovesruby
Copy link
Contributor Author

I don't consider myself a poweruser of bootstrap or bootstrap_form, so I'm not sure. But the only thing I found now was grouped_collection_select, which you mention in the linked issue.
At least for the selects there are examples in the Bootstrap 5 documentation: https://getbootstrap.com/docs/5.0/forms/input-group/ . And the "ordinary" select of bootstrap_form already handles prepend and append :)

@lcreid
Copy link
Contributor

lcreid commented Jun 1, 2022

Great. Thanks for looking into this. I'll merge this shortly.

Thanks for your patience. I don't actually use this gem at the moment, so it always takes me a while to get back up to speed. Thank you for you support and efforts to improve the gem.

Copy link
Contributor

@lcreid lcreid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. It looks good. We really appreciate your contribution.

@lcreid lcreid merged commit 06ca84a into bootstrap-ruby:main Jun 1, 2022
@judylovesruby
Copy link
Contributor Author

Thanks for merging this @lcreid! I saw you also did some other changes lately. Do you plan to release more changes together?

@lcreid
Copy link
Contributor

lcreid commented Jun 25, 2022

I was trying to get another change in, but obviously I haven't managed that yet. I'll try to do a release soon, but in the meantime you can temporarily change your Gemfile to use this:

gem "bootstrap_form", git: "https://github.com/bootstrap-ruby/bootstrap_form.git", branch: "main"

Thanks for your patience, and your PR. We really appreciate your contribution.

@judylovesruby
Copy link
Contributor Author

Okay, thanks for letting me know 🙂
I usually prefer not to include links to repositories in the Gemfile, since then bundler-audit will not be able to warn me in case of CVEs. But I can do it as temporary workaround.

@lcreid
Copy link
Contributor

lcreid commented Jun 26, 2022

That's a good point. I managed to release version 5.1, including your PR, yesterday. Thanks again for your contribution!

@judylovesruby
Copy link
Contributor Author

Thank you 🙂

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.

2 participants