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

Finalise (and Release) Bootstrap 4 Support #732

Closed
carltongibson opened this issue Aug 11, 2017 · 19 comments
Closed

Finalise (and Release) Bootstrap 4 Support #732

carltongibson opened this issue Aug 11, 2017 · 19 comments

Comments

@carltongibson
Copy link
Collaborator

carltongibson commented Aug 11, 2017

As of 10 Aug 2017 Bootstrap 4 has moved from "Alpha" to "Beta".

This should bring the stability we've been missing:

Long story short, shipping a beta means we’re done breaking all your stuff until our next major version (v5). We’re not perfect, but we’ll be doing our best to keep all the classes, features, and docs URLs as they appear now in this release.
— Announcement Blog Post

I'm not using Bootstrap myself currently — so I need help with this!

I'm going to close all other bootstrap 4 issues and will take PRs Ref #732 helping to bring the Bootstrap 4 support up to scratch.

I'm planning a new release of Crispy Forms in the Autumn — before Django 2.0. If we get good input here I will bring that forward.

Thanks for the input everyone!

@zoidyzoidzoid
Copy link
Contributor

Sorry for my silence over the last couple months, I haven't been spending as much time as I'd like on open source work.

I've updated crispy-test-project to use the beta, and it'd be great to flesh that out to cover all the issues people have mentioned.

I'm gonna try make my way through some of the issues to see if they're still a problem.

@carltongibson
Copy link
Collaborator Author

@zoidbergwill:

First, no stress! OSS is a labour of love and often other things take priority. Anyone who doesn't understand that needs to reassess. ❤️

Second: Great! It's time for a new version of CF and if we can get BS4 compat finalised (bar the inevitable edge-cases) that would be a great addition.

@zoidyzoidzoid
Copy link
Contributor

You're right. Thank you. 🙏

Awesome. That's exciting.

I think we wanna get similar changes to all of these in:

#660
#661
#716

@zoidyzoidzoid
Copy link
Contributor

zoidyzoidzoid commented Aug 22, 2017

File inputs look pretty bad, but I don't know if that's a regression:

image

They should look more like:

image

Ours:

<input type="file" name="photo" class="clearablefileinput" id="id_photo">

What it should be closer to:

<label class="custom-file">
  <input type="file" name="photo" id="id_photo" class="custom-file-input">
  <span class="custom-file-control"></span>
</label>

@zoidyzoidzoid
Copy link
Contributor

Should we make separate issues under a milestone?

@carltongibson
Copy link
Collaborator Author

carltongibson commented Aug 22, 2017

@zoidbergwill Up to you: I'd be happy just to go straight to PRs with Ref #732 and a description. (Screenshots are good!)

@carltongibson
Copy link
Collaborator Author

@zoidbergwill If you want to create fresh tickets (on the milestone) with the "Help Wanted" flag, we may get some input...? 🙂 — It would lighten the load.

@zoidyzoidzoid
Copy link
Contributor

Sounds like a great idea. 👍

@zoidyzoidzoid zoidyzoidzoid modified the milestones: 1.7, Next Release Aug 22, 2017
@carltongibson carltongibson removed this from the 1.7 Release milestone Oct 17, 2017
@yunti
Copy link

yunti commented Oct 19, 2017

Currently for select input I'm seeing the default browser select rather than the bootstrap 4 .custom-select. The custom-select looks a lot more consistent across browsers so would be a better default but subject to opinion. If we go with the default way it would be good to have an easy way to override. I presume currently we would need to override the field template and create a custom version with a condition for {% if field|is_select%}?

@carltongibson
Copy link
Collaborator Author

@yunti progress is a bit stalled at the moment. If you want to contribute to get BS4 support finalised that would be very welcome!

@zoidyzoidzoid
Copy link
Contributor

I'm pretty sure we want the bootstrap select. That sounds like a regression.

We only have an example multiselect in the crispy test project but it looks right

@zoidyzoidzoid
Copy link
Contributor

I've been on holiday, but I'm back now. 👋

@yunti
Copy link

yunti commented Nov 17, 2017

Currently errors don't appear to be working with bs4 beta 2 (or beta) but they do work with alpha 6.
(posting this here rather than a separate issue, but let me know if you would prefer separate).

This example form:
https://gist.github.com/yunti/8c091cd446082eb1208e076594fc2439

Generates this output, which uses bs4 beta 2 and shows missing error:
https://codepen.io/anon/pen/GOOdmM

Same output but with bs4 alpha 6 which has error displayed:
https://codepen.io/anon/pen/JOOvyG

update: its a bs4 bug twbs/bootstrap#23454

@zoidyzoidzoid
Copy link
Contributor

I'm not sure what exactly we need to document other than mentioning we support bootstrap 4 in more places, and that adding form-horizontal as a form_class will add row classes to make forms work as expected in Bootstrap 4.

@jmooo
Copy link

jmooo commented Feb 4, 2018

It looks like input-group-addon was removed in the final B4 release, and now uses input-group-append and input-group-prepend instead, I believe that's why my fields generated with PrependedText look strange now anyway. Also the those prepend/append suffixes look like they go in a div now instead of span

https://getbootstrap.com/docs/4.0/components/input-group/
vs
https://v4-alpha.getbootstrap.com/components/input-group/

@zoidyzoidzoid
Copy link
Contributor

I've made a PR: #771

I will test it in the morning properly though, and test it on crispy-test-project.

@jmooo
Copy link

jmooo commented Feb 4, 2018

Wow that was fast, glancing at the PR there may be some other details to getting those input boxes working as in the alpha releases. Such as the prepend/append div now has a new span nested under it with the class=input-group-text which looks to be required to get the gray background and rounded corner styling?

@zoidyzoidzoid
Copy link
Contributor

Whoops, crispy-test-project was not using Bootstrap's stable release. Should be all happy now.

@carltongibson
Copy link
Collaborator Author

Version 1.7.1 is available on PyPI now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants