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

Support Bootstrap 5 #177

Open
vstollen opened this issue Nov 15, 2021 · 6 comments
Open

Support Bootstrap 5 #177

vstollen opened this issue Nov 15, 2021 · 6 comments
Labels
bundled-assets JS and CSS Issues

Comments

@vstollen
Copy link
Contributor

With Blacklight adding Boostrap 5 support in release v7.20.0, Bootstrap 5 support for the range limit would be nice as well.

@jrochkind
Copy link
Member

@vstollen Do you have any sense of what that would look like, what would need to be changed for Bootstrap 5? If you did, or wanted to figure it out, that would probably help make this more likely! Even just sharing what you've noticed not working in Bootstrap 5!

I think Blacklight currently supports BOTH Bootstrap 4 and 5 somehow, I'm not sure of the details. So I guess this has to be the same.

@vstollen
Copy link
Contributor Author

I'm not sure, if I got everything, but here is what I've found:

  1. sr-only is now visually-hidden
  2. input-group-append is not needed anymore
  3. The close button is broken. The same applies for blacklight itself, so I've created an issue there: Modal "close" button broken in Bootstrap 5 blacklight#2518

In our installation, this meant:

  1. Adding visually-hidden in:
  2. Removing the div in line 64 of _range_limit_panel.html.erb
  3. Applying the fixes proposed in Modal "close" button broken in Bootstrap 5 blacklight#2518 for lines 3 and 4 in range_limit_panel.html.erb

The only problem I see in this is, that the div I removed in 2. is needed in Bootstrap 4, but seems to break the layout in Bootstrap 5. But maybe there is another solution for that?

I can submit a draft pull request with these changes (maybe except 2.) later.

For reference: Bootstrap Documentation, Migrating to v5.

@vstollen
Copy link
Contributor Author

Maybe we could do something for 2. with the sr-only and visually-hidden classes.

We could add both versions and sr-only would hide the BS4 implementation, visually-hidden would hide the BS5 implementation. To not affect accessibility too much, we could add aria-hidden to one of the two implementations.

@jrochkind
Copy link
Member

I am not using Bootstrap 5 yet and haven't looked into it, but I can test any PR you make on my Bootstrap 4 app.

vstollen added a commit to vstollen/blacklight_range_limit that referenced this issue Nov 15, 2021
The changes should keep compatibility with Bootstrap 4.

See: projectblacklight#177
@vstollen
Copy link
Contributor Author

@jrochkind I've created the PR, but only tested it in our Bootstrap 5 app.

@jrochkind
Copy link
Member

Thanks @vstollen , linking to the PR is convenient, looks like it's #178.

I'll test it in my Bootstrap 4-using app presently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bundled-assets JS and CSS Issues
Projects
None yet
Development

No branches or pull requests

3 participants