mistic100 / jQuery-QueryBuilder

jQuery plugin offering an interface to create complex queries
https://querybuilder.js.org
MIT License
1.67k stars 552 forks source link

Bootstrap 5 Further Progress #992

Closed liamkeily closed 5 months ago

liamkeily commented 5 months ago

This PR carries on from https://github.com/mistic100/jQuery-QueryBuilder/pull/988 and https://github.com/mistic100/jQuery-QueryBuilder/pull/973 to take this a bit further.

Merge request checklist

Screenshots Before

image image

Screenshots After

image image
liamkeily commented 5 months ago

@mistic100 is the CI here failing for a known reason?

mistic100 commented 5 months ago

it says it cannot get bootswatch-dist, but this package is for Bootstrap 3 anyway, you probably should remove it

mistic100 commented 5 months ago

By the way there are now three BT 5 pull requests @stephencmorton @dethell @liamkeily are you all wroking on this ? Do you want direct access to the repository to work on a single branch here ?

liamkeily commented 5 months ago

@mistic100 yea i've picked up from where the others left off, it is a bit awkward so a bootstrap-5 branch that we can all commit to could be useful

liamkeily commented 5 months ago

This branch feels like its in a fairly good place now, although needs further testing.

bootstrap-select does not appear to work for Boostrap 5 so i have commented it out for now. chosen-selectpicker appears to act as an alternative and does work but does not really fit the Bootstrap 5 styling so i have left it disabled for now. The query builder appears to work nicely without either so I'm not sure how much functionality we are missing without it.

Feedback / testing would be appreciated from @stephencmorton @dethell if you get any time. I will also test bringing this branch into the project i use it for to test it further.

dethell commented 5 months ago

@liamkeily I'm fine with the work continuing here, although if @mistic100 want's us to be committers to a specific branch that's fine too. Whichever works. I'll go ahead and pull this branch and retest.

dethell commented 5 months ago

This looks good. Checkboxes work, dark mode works, selects work. I can't see any reason not to merge this. Back to you @mistic100 .

liamkeily commented 5 months ago

Thanks for reviewing @dethell

One thing i've wondered is how this will play with the documentation site at https://querybuilder.js.org/demo.html which still uses Bootstrap 3.

stephencmorton commented 5 months ago

I'm fine with any branch as well. If this one is a superset of all the others, I'm good with that.

stephencmorton commented 5 months ago

Oh, the "SRBuilds" id on some of my commits was just me accidentally logged in as my work account. I thought about it and it's ok, I do not need to "git commit --amend --reset-author" or anything like that, my work would be fine with it and it does not reveal I should not. I actually started this because I investigated using jquery-querybuilder for an internal tool.

liamkeily commented 5 months ago

@mistic100 Do you have any information about how this project is built and published to npm? I wanted to replicate it as closely as possible to make sure it all works but i'm a bit confused by the setup.

I just noticed that the dev branch package.json still contains an older version "2.5.0" than the latest release "2.7.0".

mistic100 commented 5 months ago

That's because the release is done from the master branch. release

To do a release :

mistic100 commented 5 months ago

If I had to really work on this lib (I won't) I would reuse the workflows I created for my other library https://github.com/mistic100/Photo-Sphere-Viewer/tree/main/.github/workflows

liamkeily commented 5 months ago

I replicated the release process and have been testing this on a real project and fixed a few more minor bugs. It's shaping well, just want to do a bit more testing then might be time to start thinking about tagging up a 3.0.0 pre-release.

What do you think @mistic100 ?

stephencmorton commented 5 months ago

Thanks so much @liamkeily . Yeoman's work here. It looks like you removed selectpicker and then re-added it. For a clearer history would it be possible to remove those commits, or if there's not a net-null result, at least squash them into whatever the net change is.

liamkeily commented 5 months ago

@stephencmorton yea i'll get rid of those commits for now. I removed it thinking we could just drop it, but then i realised that the demo itself relies on bootstrap-select for language selection.

stephencmorton commented 5 months ago

(We could just change the demo, if that makes the most sense.)

mistic100 commented 5 months ago

Go ahead, squash merge the PR into dev and I'll publish a pre-release to npmjs.org

liamkeily commented 5 months ago

(We could just change the demo, if that makes the most sense.)

I've gone ahead and removed the bootstrap-select from all places. Although it may be a nice to have for some users, It doesn't appear to be an essential part of the builder and i'm struggling to get it to work correctly. The chosen-selectpicker plugin is also still available for anybody that wants select box filtering functionality.

I've also updated screenshots above