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

Bootstrap v5 #7411

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open

Bootstrap v5 #7411

wants to merge 23 commits into from

Conversation

smithdc1
Copy link
Contributor

@smithdc1 smithdc1 commented Jul 12, 2020

Fixes #7327

This is a WIP patch to upgrade the templates to Bootstrap v5. I've tried to chunk the commits up to help review, this will change as this will quite a number of files and lines of code.

I feel like I've made good progress so far. Only major issue I've found so far is tooltips on the dropdown button. The solution on bootstrap 4 (which I've used here) no longer works on bootstrap 5. Another alpha release is due of bootstrap 5 shortly, if it is not fixed in that I can try approaching the issue tracker.

To do (there is much more than this, but here are things I've changed but need to go back and look at):

  • Fix tooltip on split dropdown button
  • Change the js file to the bundled version (can then drop link to popper.js)

@carltongibson
Copy link
Collaborator

Hi @smithdc1. Thanks for this! Super start.

@smithdc1
Copy link
Contributor Author

Thanks 😄

It's coming along well, currently wrangling with the collapse menu in the docs.

Got my eye on the upcoming releases, seems like they made good progress on alpha 2 in the last few days. I can already see some breaking changes in the next release but they should be incremental changes to the current changes. (e.g. rename of sr-only)

@smithdc1
Copy link
Contributor Author

So I'm fairly pleased with where I've got this to. It's a bit of a waiting game now to see the new releases come out and see if there are any more breaking changes, and to see if the drop-down tool-tip gets fixed. Can also review the remaining css in bootstrap-tweaks and default to see if any further rationalisation can take place here.

One question I do have is for the filter form. The filter form looks a bit odd, it looks like a template from django-filter, what's going on here? (I think I know where this is headed... 😄 )

@carltongibson
Copy link
Collaborator

One question I do have is for the filter form. The filter form looks a bit odd, it looks like a template from django-filter, what's going on here? (I think I know where this is headed... 😄 )

Chuckle. 😀

Erm... the form is generated by django-filter but is it rendered with crispy forms? (A while since I looked in here...) If you throw up a few links to exact places I'm sure I can get the relevant grey-cells lined up again. 😝

@smithdc1
Copy link
Contributor Author

Ok - let's leave it some time and I'll dig deeper at some point in the coming months. There is going to be plenty of time before Bootstrap 5 reaches a stable release.

There may be a crispy-bootstrap-5 template pack by then (template work is done, I need to find the effort to put it into a package, write docs, setup c/i etc)

@smithdc1
Copy link
Contributor Author

Note (to self really).

#30170 on bootstrap 5 changes feedback messages. Html layout and CSS classes will need to be changed.

Currently this ticket is on the inbox for v5 alpha 2.

@joshjung
Copy link

Looking forward to this!

@smithdc1
Copy link
Contributor Author

Looking forward to this!

Thanks -- It's coming but bootstrap 5 development itself is steadily paced. For your need I'm not sure I have a good answer, maybe you could have your own fork with this work, noting it is un-reviewed (but if you're in the market to have a look 😉 ) I'm not sure an alpha version is better than an old one from a security perspective 🤷 .

@smithdc1
Copy link
Contributor Author

smithdc1 commented Dec 7, 2020

Bootstrap5 beta 1 is now out. I'll be looking to test my memory by revisiting this soon ™️.

* navbar-expand-md to stop navbar growing when dropdown opens.
* Split dropdown doesn't work with tooltip - looks like a bs5 bug (same layout works with bs4)
* Need to check button colours
* Need to look at colour of dropdown links
* BS3 had lots of top (20px) and bottom (10px) margin to push page down. Now added top margin on the div.
* Font has style has changed with bs5. But is slightly custom as font weight is over riden. Personally think it looks ok.
* form-horizontal is no longer used. Need to use the grid.

aligned tabs right

Raw Data Form
* Use grid / rows to space form. form-group has been deprecated in favour of utility classes.

Replaced `well` class (background colour/border) with bootstrap utility classes
* form-group is deprecated in bootstrap 5 and is replaced by utility classes.
* checkbox are now fully custom by default in Bootstrap5 - removed previous tweaks.
* control-label --> col-form-label
* errors now need `invalid-feedback` on the text and `is-invalid` on the input
* new `form-text` class for help text
* New html layout and css classes for radio/multiple checkbox. Added work around for errors and inline (see comments in code).
* error class has changed to `invalid-feedback`
* Used bootstrap v5 icon pack
* Updated glyphicons to Bootstrap 5 icons
* Fixed sidebar collapse (links now work)
* Changed fontawsome icons to bootstrap icones
* render_markdown returns its content in a <p> tag, so don't need to wrap again.
* No need to add popper.js as a separate source
The hide class in bootstrap 2 added display:none.

This is done via d-none in v5 see.

https://getbootstrap.com/docs/5.0/utilities/display/#notation
@tomchristie
Copy link
Member

So I think the obvious things that need fixing are...

  • The button sizes after this change are significantly bigger, and they don't fit the layout so well.
  • The changed + and x icons for the create/delete buttons are just a bit blergh.
  • The brighter palette perhaps doesn't suit the rest of the interface style quite as well, but I don't think that's a massive problem.

@lucasdavid
Copy link

Breadcrumbs are quite different too. Was that intentional? I think .bg-light .rounded classes could be added here.

@tomchristie tomchristie modified the milestones: 3.13 Release, 3.14 Dec 13, 2021
{% if field.label %}
<label {% if style.hide_label %}class="sr-only"{% endif %}>{{ field.label }}</label>
<label class="form-label {% if style.hide_label %}sr-only{% endif %}">{{ field.label }}</label>
Copy link

@ghost ghost Feb 12, 2022

Choose a reason for hiding this comment

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

I think in Bootstrap 5 .sr-only was removed: https://getbootstrap.com/docs/5.0/getting-started/accessibility/#visually-hidden-content

The reason I am noticing this is because when I set hide_label to True in my serializer field I saw the class is added but when I looked in the CSS it was not there.

So anywhere in any of the forms it uses .sr-only which I think is a lot of templates would have to swap out to the v5 class.

@ghost
Copy link

ghost commented Feb 12, 2022

I left a comment

I think anywhere there is .sr-only that has to be removed and the v5 class used.

@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 16, 2022
@auvipy auvipy removed the stale label Dec 24, 2022
@auvipy
Copy link
Member

auvipy commented Mar 1, 2023

as this is staled, should we try a smaller incremental version with bootstrap v4 instead?

@baseplate-admin
Copy link
Contributor

as this is staled, should we try a smaller incremental version with bootstrap v4 instead?

or maybe we should have our own CSS like django-admin 👀

@stale
Copy link

stale bot commented Jun 10, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added stale and removed stale labels Jun 10, 2023
@auvipy
Copy link
Member

auvipy commented Jun 13, 2023

we can revamp it with newer https://blog.getbootstrap.com/2023/05/30/bootstrap-5-3-0/ which has new colors and many improvements.

@auvipy auvipy removed this from the 3.15 milestone Jul 11, 2023
@stale
Copy link

stale bot commented Sep 17, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Sep 17, 2023
@baseplate-admin
Copy link
Contributor

Hi, can this be rebased?

@stale stale bot removed the stale label Nov 7, 2023
@luanfonceca
Copy link

Hi, @smithdc1

I want to help you finish this PR. I have extensive experience with Django templates and Bootstrap 5 (I just did a similar migration on a client from v3 to v5.3).

I plan to pull your branch, update this to Bootstrap v5.3, and keep going page by page, ensuring everything works seamlessly.
In the best scenario, we would do this incrementally, keeping the existing Bootstrap v3 stylesheets, js, and jQuery in use while we're updating each piece, but as this is an open-source library, I don't think we could do this.

Are you available for a quick async talk on the IRC channel, or do you prefer to discuss the next steps here?

@smithdc1
Copy link
Contributor Author

Hi @luanfonceca -- I'm afraid I don't have time to work on this patch right now. It's been ~3 years since I've looked at this so I'd be starting from scratch to pick this up again. I think it would be amazing if you have capacity to update and address the review comments!

@smithdc1 smithdc1 closed this Dec 15, 2023
@luanfonceca
Copy link

Yeah, there's no need to worry about it.
I had that in mind already.

Thanks

@auvipy auvipy reopened this Dec 16, 2023
@luanfonceca
Copy link

Hi @auvipy, I can take over this migration, I saw some conversation about migrating to v4 before trying the v5, do you know how that's going?

Last week did some attempts and can share where I'm at with you.

@luanfonceca
Copy link

Here's a short video showing how far I got with this last week; I had a couple of issues, and the layout still needs some improvements.

I can open a new issue up for discussion, along with my ideas about this, as there were several attempts related to this migration. I think we need time to decide what we want to do and how, as well as the impact of this migration on third-party apps that use the DRF browseable API page.

Screen.Recording.2023-12-16.at.12.06.34.PM.mov

@martinberoiz
Copy link

Could this maybe be released as some alpha version for some future release? Or is it too risky? I'm interested in BS5 too

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.

Update to Bootstrap 4.5.0