Problem/Motivation

Yeah 4.2.1 is out we should update ASAP https://blog.getbootstrap.com/2018/12/21/bootstrap-4-2-1/.

CommentFileSizeAuthor
#11 3024839-bump_bootstrap_to_4_3_1-11.patch64.8 KBytsurk
#8 3024839-bump_bootstrap_to_4_3_1-8.patch263.16 KBytsurk
#6 3024839-bump_bootstrap_to_4_3_1-6.patch1.2 KBytsurk

Comments

pivica created an issue. See original summary.

pivica’s picture

pivica commented
Issue summary: View changes
pivica’s picture

pivica commented

Only 4 open issues for Bootstrap 4.3 currently https://github.com/twbs/bootstrap/issues?q=is%3Aopen+is%3Aissue+project%.... When starting with this issue lets consider going to 4.3 if possible.

pivica’s picture

pivica commented
Title: Update to Bootstrap 4.2.1 » Update to Bootstrap v4.3.1
Issue tags: +next-release

v4.3.1 is released, updating issue.

pivica’s picture

pivica commented
Issue tags: -next-release +next-release-planned
ytsurk’s picture

ytsurk
Location Switzerland
commented
StatusFileSize
new 3024839-bump_bootstrap_to_4_3_1-6.patch1.2 KB

This update should be quite seamless. The bootstrap change log lists no incompatibilities.

A XSS issue was solved just in 4.3.1, and there is the following deprecation:

img-retina, invisible, float, and size mixins are now deprecated and will be removed in v5.

Keep in mind that the composer files of the projects should also be updated ! (and also the composer.json attachment on this page: https://www.drupal.org/docs/8/themes/bs-base/quick-start

ytsurk’s picture

ytsurk
Location Switzerland
commented
Status: Active » Needs review
ytsurk’s picture

ytsurk
Location Switzerland
commented
StatusFileSize
new 3024839-bump_bootstrap_to_4_3_1-8.patch263.16 KB

Totally forgot to compile the CSS ..

Anyway - we should wait for #3024835: Update gulp tooklit and then re-roll.

pivica’s picture

pivica commented

Yeah, let us wait #3024835: Update gulp tooklit I think it makes sense we commit CSS to build changes also with that issue so we can then more easily track CSS changes from Bootstrap update. Does this make sense to you @ytsurk?

ytsurk’s picture

ytsurk
Location Switzerland
commented

Yes - fine with me.

ytsurk’s picture

ytsurk
Location Switzerland
commented
StatusFileSize
new 3024839-bump_bootstrap_to_4_3_1-11.patch64.8 KB

Here the patch against latest 8.x-1.x with compiled CSS.

The CSS changes are not only related to the bootstrap update ..

ytsurk’s picture

ytsurk
Location Switzerland
commented
pivica’s picture

pivica commented

> The CSS changes are not only related to the bootstrap update ..

Commit ab09731 has recompiled bs_bootstrap against all latest changes, so this patch should hold only changes related to bootstrap update, right?

pivica’s picture

pivica commented

Beside just upgrading and recompiling against latest Bootstrap we should check new features in 4.2 and 4.3 releases and if we like something create follow-ups for implementation.

From v4.2.0 release next stuff looks interesting:

- New: Added a new spinner loading component.
- New: Added new toast component for displaying notifications.
- New: Added a new iOS style switch (a modifier class to our custom checkboxes).
- New: Validated form fields now have feedback icons on :invalid and :valid fields. Disable them with the $enable-validation-icons boolean Sass variable (defaults to true).

From v4.3.0 release next stuff looks interesting:

- Nothing for now.

Any ideas, wishes, preferences?

sasanikolic’s picture

sasanikolic commented

- I am up for updating/implementing the new spinner component.
- I like the notifications, but I don't think we're using them regurarly. Maybe would be a good improvement to switch from standard Drupal notification bar to use this new notification toast? Although if texts are long in that notification popup it might not look good.
- I don't think it's necessary to implement the iOS switch since it might not get along with other design patterns?

ytsurk’s picture

ytsurk
Location Switzerland
commented

Actually you're right. This is only the changes introduced by the new bootstrap version. Hadn't it on the map that you re-compiled too.

  • I'm totally for replacing the good ol throbber with the grow spinner :D. The code is already there in this customer project ;)
  • I find notifications disturbing and would not replace the drupal messages with them. But they could be used for AJAX reactions imo.
  • I would vote for the switches thoe. Disablable via variable, if a project design does not fit them.
  • The .is-invalid style should be used for replacing the actual inline form errors. The .is-valid has no use case right now, as we do not validate form input via AJAX right now ..

From 4.3 I see:

  • .text-break could be implemented, as right now text-content overflow will be hidden - maybe this is wanted ?!
  • To use the two different roundings could be used as style element. No concept right now .. just saying

  • pivica committed 79de610 on 8.x-1.x authored by ytsurk 
    Issue #3024839 by ytsurk, pivica: Update to Bootstrap v4.3.1
pivica’s picture

pivica commented
Status: Needs review » Active 
+++ b/themes/bs_bootstrap/bs_bootstrap.bs_base.install
@@ -1,5 +1,26 @@
 <?php
...
+/**
+ * @file
+ * Update hooks for bs_bootstrap base themes.
+ */
...
+<?php

Patch got corrupted somehow - duplicate <?php and comment. Fixed that and moved 8001 update to the bottom.

Tested this against custom themes, works great. Committed.

Will leave this issue open until we create all the follow-ups that we want.

ytsurk’s picture

ytsurk
Location Switzerland
commented

Here the introduction of the spinner :D #3047339: Introduce bootstrap's spinner as throbber

sasanikolic’s picture

sasanikolic commented

Not too sure about the patch above. What happens if the child theme is version 4.0.0. as in one of our cases? Then there's no way the child theme will get updated as the update is too specific (~4.1.3 to ~4.3.1). Maybe we'd need a more generic check?

ytsurk’s picture

ytsurk
Location Switzerland
commented

Where still in -alpha ;) But you're kind of right, even more when updating SASS dependencies .. #3024835: Update gulp tooklit.

Also one maybe want's to be able to use other versions of "some libs". Or should bs_base preset the toolset fix?
From now on, if the update-hooks are run in order, and nothing was changed, everything should actually be reflected.
(The last bootstrap version bump(s) had no update-hooks ..)

Anyway, manual work is also included ATM, fe. updating the Drupal instances composer.json ...

Again, we're in -alpha, and bs_base want's to be a developer theme... We have to define how far we want to go...
IMO - it's important that this is documented anywhere, so I added a sentence in the documentation: https://www.drupal.org/docs/8/themes/bs-base/theme-development/updating-... and it should be mentioned in the release notes (@ivijca): https://www.drupal.org/project/bs_base/releases/8.x-1.0-alpha6

This ticket is active to discuss what features of bootstrap v4.3.1 we want to integrate.

pivica’s picture

pivica commented

> Not too sure about the patch above. What happens if the child theme is version 4.0.0. as in one of our cases?

Yeah, it will not work, a developer would need to fix this manually. In the future, we should do a more generic replacement.

> Also one maybe want's to be able to use other versions of "some libs". Or should bs_base preset the toolset fix?

We need to have an upgrade path to tested lib versions on which we are depending on it. A developer is free to revert/change lib versions to whatever it wants after an update or write it own update hooks that are overriding ours.

pivica’s picture

pivica commented

> What happens if the child theme is version 4.0.0.

Well, we stumbled just on that case for client projects so i've created #3080540: Make sure that bootstrap is updated to 4.3.1 version with a patch that should fix this.

pivica’s picture

pivica commented
Status: Active » Fixed

Closing this in favour of #3219566: [META] Track interesting Bootstrap features, we will move all further discussion to that issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.