search

backdrop-contrib / views_bootstrap

This module provides a set of styles for the Views module. It allows you to create responsive Bootstrap 3.x components, such as grids, carousels, tabs, and tables, all within the configuration settings of the powerful Views module.
https://backdropcms.org/project/views_bootstrap
GNU General Public License v2.0
0 stars 7 forks source link

General feedback on this module #27

Open stpaultim opened 5 years ago

stpaultim commented 5 years ago

@Radcliffe and I are relatively new to maintaining modules. We took over this module because we needed a module that would help us create views with accordions (my go-to module for D7 was not available). However, I'd love to get some feedback on what you think about this module in general.

A great thing about this module is that is does a bunch of things. A problem with this modules is that it does a bunch of things.

Should we be spending time making this module great or would it be better to support smaller modules that are more focused on specific tasks (tabs, carousels, accordions, etc)?

daggerhart commented 5 years ago

This module makes a lot of sense when used in conjunction with a bootstrap based theme. But the major thing I notice that this module does differently from the D7 version is that it tries to provide the bootstrap css and js files. In my opinion, that should not be the responsibility of this module.

If I had to guess, it's the inclusion of these css and js assets that are causing all the admin-theme related problems (#4, #7, #24, more?). Theoretically, this module should only change the HTML output of a single view when that view has a views_bootstrap style plugin selected. It should be up to the theme to include the bootstrap assets.

I think if we were to remove the automatic inclusion of bootstrap assets from this module, then maintenance of the module would become significantly easier. We would likely want to include some helpful documentation about how users should go about including the bootstrap assets in their theme.

stpaultim commented 5 years ago

We have also discussed whether or not a sub-module approach would make sense? Allowing the user to only enable the specific tools that they need. I'm not really sure if that adds much. One downside of having all these features in one modules, is that you suddenly get 8 new format options in views even if you only need 1.

Moving to a submodule or removing the assets from the module would require a major new release, since I assume it would break existing sites.

yorkshire-pudding commented 2 years ago

Managed to do most of what I need - I'm struggling a bit with Carousel as I can't get it to pause on mobile. I understand this is built in to v4 but is not in v3. I note that @keiserjb has already requested that it supports Bootstrap 4 in #47

bugfolder commented 2 years ago

@yorkshire-pudding, feel like trying out the 1.x-3.x dev branch on the module? Hoping to do a release soon.

Per @keiserjb's request, because of the changes in Bootstrap 4, a BS4 version of Views Bootstrap would probably be a new branch (1.x-4.x).

yorkshire-pudding commented 2 years ago

Thanks @bugfolder I've tested this and the Bootstrap Grid and Bootstrap Carousel work the same as the current branch. One thing that doesn't is the view admin page.

image

If I add this in inspector it works:

.views-displays .label {
  color: #666666;
}

Not sure the best place for this but I've added another CSS file and added it via the module in another backdrop_add_css function for now.

bugfolder commented 2 years ago

One thing that doesn't is the view admin page

@yorkshire-pudding, that's odd, I'm not able to reproduce that on my test site. Would you (a) open a separate issue for this, (b) open Developer tools in your browser, inspect one of the white labels, and show what the CSS is that makes the label white, and which file it's in? Also, which Bootstrap version have you selected at admin/config/user-interface/views_bootstrap?

yorkshire-pudding commented 2 years ago

Ha! mystery solved. I was using 3.3.6 but updating to 3.4.1 fixes it.

bugfolder commented 2 years ago

Glad to hear that, but it still leaves a mystery: when I switch my test site to 3.3.6, I still see black labels. Oh well.

stpaultim commented 2 years ago

FYI - this may or may not be relevent. https://github.com/backdrop-contrib/views_bootstrap/issues/4

bugfolder commented 2 years ago

Ah, it happens with Auto Preview turned on in the View. Turning it off, things display normally. Now there's something to chew on.

bugfolder commented 2 years ago

Version 1.x-1.x had a CSS shim that addressed this issue. I've added a version of the shim to the 1.x-3.x branch now.

yorkshire-pudding commented 2 years ago

Thanks @bugfolder - that now seems to work with all the versions available.

keiserjb commented 2 years ago

@yorkshire-pudding, feel like trying out the 1.x-3.x dev branch on the module? Hoping to do a release soon.

Per @keiserjb's request, because of the changes in Bootstrap 4, a BS4 version of Views Bootstrap would probably be a new branch (1.x-4.x).

At this point we should probably be looking at Bootstrap 5 anyway. I run this module on sites with the Bootstrap 5 theme.

keiserjb commented 2 years ago

@yorkshire-pudding, feel like trying out the 1.x-3.x dev branch on the module? Hoping to do a release soon. Per @keiserjb's request, because of the changes in Bootstrap 4, a BS4 version of Views Bootstrap would probably be a new branch (1.x-4.x).

At this point we should probably be looking at Bootstrap 5 anyway. I run this module on sites with the Bootstrap 5 theme.

I've got the grid and the accordions working with Bootstrap 5. Trying to get the Carousel.