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

[WIP] Bootstrap 5 #32037

Merged
merged 100 commits into from
Jan 22, 2021
Merged

[WIP] Bootstrap 5 #32037

merged 100 commits into from
Jan 22, 2021

Conversation

wilsonge
Copy link
Contributor

@wilsonge wilsonge commented Jan 15, 2021

Summary of Changes

This was my dev work on bs5. It's less radical than @dgrammatiko 's work in #31990 in that it uses the official Bootstrap 5 JS combined file (although I think his work is valuable and probably should be merged in a future J minor (just doing less things at once). However largely it's using his code so has many of the same JS issues left to fix

Additionally however it uses the bootstrap 5 css and changes many of the core uses around to use the bs5 styles or in cases where I feel extensions may have been using it - backports some limited classes.

Note I do not plan on doing much more work on this. I already shouldn't have spent 3 days on this as I have more important things to be doing like release blockers :) If someone wants to pick it up feel free. If people wanna PR feel free.

Note just because it's me making this PR doesn't mean it's guarenteed to be merged - there's still a production vote on that so it's not in my hands. However given one of the major against points is that it will delay core. I would assume a completed PR by someone could influence the way some people vote...

In Depth Changes

JS Changes

All data-* attributes used by bootstrap now have a bootstrap prefix (e.g. data-toggle=modal now is data-bs-toggle=modal)

CSS Changes

CSS Class Backports in Core
  • form-group
  • sr-only
  • custom-select
  • card-columns
CSS Class Changes
  • Removed and replaced .badge modifier classes with background utility classes (e.g., use .bg-primary instead of .badge-primary. Note this doesn’t apply text colour so you may need colour-dark with bg-light etc)
  • Removed .input-group-append and .input-group-prepend. You can now just add buttons and .input-group-text as direct children of the input groups.
  • Instead of applying .btn-block to individual buttons:-
    • For individual “block buttons”, add .w-100.
    • A group of buttons now get wrapped in a parent .d-grid class and can use .gap-* utilities for spacing (in core we are using gap-2
    • Jumbotron was removed. We've replaced it with the utility classes bg-light p-3 rounded
CSS Class Changes for Bootstrap RTL Support
  • Renamed .float-left and .float-right to .float-start and .float-end.
  • Renamed .ml-* and .mr-* to .ms-* and .me-*.
  • Renamed .text-left and .text-right to .text-start and .text-end.
  • Renamed .dropdown-menu-*-left and .dropdown-menu-*-right to .dropdown-menu-*-start and .dropdown-menu-*-end.

There are also several SCSS mixin changes in a similar vein - but these shouldn't be relevant to 3rd party extensions

Other changes

  • Toolbar dropdowns are now located within the toolbar-button if the selector is the button itself. The 'toggle split' dropdowns used for save groups are unaffected
  • Several backend modules that were importing the bootstrap framework without using any JS modules (e.g. mod_feed, mod_latest, mod_latestactions, mod_logged, mod_popular, mod_user (changed to proxy through HTMLHelper::_('bootstrap.dropdown')) ).
  • Workflow Layouts now explicitly call
  • The admin stats module has had it's jQuery converted to Vanilla JS (thanks @dgrammatiko )
  • SCSS Only Change: The media-breakpoint-down() uses the breakpoint itself instead of the next breakpoint. Use media-breakpoint-down(lg) instead of media-breakpoint-down(md) to target viewports smaller than the lg breakpoint.
  • SCSS Only Change: Removed hover, hover-focus, plain-hover-focus, and hover-focus-active mixins. Use regular CSS syntax for these moving forward.
  • Reverted to use bootstrap's native modal close header instead of overriding our own which now looked out of place
  • Various places in core have removed jQuery dependencies when dealing with modals and have moved to vanilla JS
  • Fixed styling class for the range form field layout
  • Form inline classes have been backported
  • Fixed missing bootstrap dependency in smart search

Testing Instructions

Check every html view in the cms :)

Documentation Changes Required

Yup

@wilsonge wilsonge marked this pull request as draft January 15, 2021 01:06
@Ruud68
Copy link
Contributor

Ruud68 commented Jan 15, 2021

Hi George,
Just out of curiosity: the impact of switching back-end framework in 'the last' beta (from BS4 to BS5) mainly impacts extension developers. Especially the ones who did the hard work of migrating all their extensions to J4. It will also impact trainers who started to make instructions videos and documentation based on the BS4 back-end.
For me the 'Joomla community' is (unlike the Open Source code) very 'closed' when it comes to voting and having a say in some very important matters for the 'real' Joomla community (the ones using it or building on top of it).
My question is: who is 'production' that is supposed to vote and are extension developers and 'trainers' who are not considered part of the Joomla 'community' able to have a equal vote in this?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32037.

@brianteeman
Copy link
Contributor

@wilsonge is referring to the people listed https://volunteers.joomla.org/departments/production

@Ruud68
Copy link
Contributor

Ruud68 commented Jan 15, 2021

@wilsonge is referring to the people listed https://volunteers.joomla.org/departments/production

Thanks @brianteeman,
any insight as to how they are going to reach out to the Joomla community for this important change (not saying we should or should not do this move, just want the Joomla Community to have a vote in the matter)

@Fedik

This comment has been minimized.

@ghost
Copy link

ghost commented Jan 15, 2021

fedik best is to test @dgrammatiko PR #31990 for Bootstrap 5

@brianteeman
Copy link
Contributor

@Ruud68 As you can see I am not in that list so I really don't know but I wouldnt expect any "reaching out"

@Ruud68
Copy link
Contributor

Ruud68 commented Jan 15, 2021

@Ruud68 As you can see I am not in that list so I really don't know but I wouldnt expect any "reaching out"

Thanks, learning from history I wouldn't expect it as well, that is why I am asking. This will for sure result in valuable community members to 'vote' with their 'feet' (dutch saying :))
I have my hopes on the Core developers (in or not in the production team) to do what is needed. Never had 'management' made me do anything that I wasn't fully committed to doing my self :)

@Magnytu2
Copy link

Hello, I know that I am not a developer, but a simple user who gives some of my time to Joomla. But not choosing to use Bootstrap 5 is like an automaker refusing to go hybrid or electric and questioning whether the network is ready. For once, after refusing the Khonsu template, the development team needs to be more visionary and ahead of its time. It is also a strategy for the future and the development of Joomla on the CMS market. It's also marketing!

@Ruud68
Copy link
Contributor

Ruud68 commented Jan 15, 2021

Hello, I know that I am not a developer, but a simple user who gives some of my time to Joomla. But not choosing to use Bootstrap 5 is like an automaker refusing to go hybrid or electric and questioning whether the network is ready. For once, after refusing the Khonsu template, the development team needs to be more visionary and ahead of its time. It is also a strategy for the future and the development of Joomla on the CMS market. It's also marketing!

I fully agree on this, my concern is with the part of not having a vision and mission you can agree on and not having the process in place to involve the relevant 'community members' in this.
To many valuable developers are walking away or putting stuff on hold.

@wilsonge
Copy link
Contributor Author

wilsonge commented Jan 15, 2021

We are well aware. There are extension dev's involved here #31765 (if you haven't commented there I strongly recommend you do as we are monitoring that closely) and also Production have a separate A4 document of Pro's/Con's that @marcodings has been working on as a result of latest Production meeting on Tuesday when we first discussed this as a result of the open discussion. We are also consulting with JED about the number of extensions that have already been marked as J4 compatible to get a feeling. As well as the main reason I started on this was to get a feel of the work involved in converting some of the core extensions.

So I can promise you that this is being taken into account @Ruud68 - because I also understand that even if BS5 doesn't impact the CMS release time because people are willing to work with on this conversion it makes no difference if extensions are going to need 2 months after the J4 stable release to become compatible again

@Ruud68
Copy link
Contributor

Ruud68 commented Jan 15, 2021

Thanks @wilsonge , I just commented (#31765 (comment)) :)

@dgrammatiko
Copy link
Contributor

It's less radical than @dgrammatiko 's work in #31990 in that it uses the official Bootstrap 5 JS combined file (although I think his work is valuable and probably should be merged in a future J minor

Just a comment here about #31990:

  • As it is atm ( 7d03afe ) it's not B/C but it can be patched to become.
  • I started that as a clean slate just to showcase the amount of work needed to do the transition (to vanilla not just v5 as George is doing here) [4.0][WIP] BS5 JS #31990 (comment)
  • This PR and [4.0][WIP] BS5 JS #31990 are not exclusive, you can have the markup/css from this one and the JS from the other (assuming the B/C breaks are patched).

In sort my take on the transition is simple: Move to BS5 (the vanilla js version) and patch the B/C breaks (doable) for any of the areas: SCSS, JS the project needs to keep B/C (personally I'll say it's better to break both at the major version). Postponing some of the needed work to a later day is not a very pragmatic approach. Bite the bullet and DO the work NOW.

Lastly I think the project needs a clearly defined vision on where it goes, eg: do we really want to ship our UI based on jQuery in 2021? In the other PR the UI is modular which in most views result to over, at least 50% less JS (for the same interactivity). Why would you punish the performance (and by extension SEO, etc) by keep on doing things the way it was done 10 years ago? (Rhetorical questions, I don't expect any answer here, I have been talking about all these matters the last 5 years and it seemed that the devs are not willing to evolve to the current state of the web. But then again if the project doesn't evolve it becomes irrelevant and finally dies).

@HDInfautre
Copy link

Hi
Formerly a developer, now a simple user, I wouldn't dare say whether to integrate it or not.

In contact with developers who sell extensions, I'll just say that we (them, me) are waiting impatiently for the new version. I'm afraid of a new delay that could make lose more members of the community than the absence or presence of this feature when the final joomla4 version is released.

@ghost
Copy link

ghost commented Jan 15, 2021

HDInfautre please do not hijack the topic and waste our time, but better go and test wilsonge or @dgrammatiko PR #31990 for Bootstrap 5

Remember topic is [WIP] Bootstrap 5

@joeforjoomla
Copy link
Contributor

With the official decision now in place, i think that it's fundamental to merge as soon as possible in a new Beta 7 and inform all developers through a JED newsletter to start to update their extensions.

@alikon
Copy link
Contributor

alikon commented Jan 22, 2021

maybe merge both PRs and release immediately another beta?

sure merge both pr's 1st....
it's really time expensive right now to test both

merging we will gain more tester ect

@wilsonge
Copy link
Contributor Author

The plan is now for an additional beta once we've stabilised the bs5 stuff. I've not talked to marketing yet (decisions finalised and blog posts have gone out today whilst I've been at work), but I'd imagine probably either next week or the week after.

sure merge both pr's 1st....

No because there can be issues from bs5 and issues from compiling bootstrap javascript ourselves. I want to do one then the other so we know the source of each bug.

@dgrammatiko
Copy link
Contributor

compiling bootstrap javascript

Glad to report, I'm just cleaning the branch before pushing it:
Screenshot 2021-01-22 at 20 49 32

@alikon
Copy link
Contributor

alikon commented Jan 22, 2021

I didn't intend to release a new beta....
just have a single pr

@PhilETaylor

This comment was marked as abuse.

@Magnytu2
Copy link

Sorry, but what testers and developers need is a plan with dates. Beta 7 for 3 weeks, PR1 for three weeks, ... and above all stick to it to regain everyone's confidence. And don't imagine that Joomla 4 will be released in 2022.

@alikon
Copy link
Contributor

alikon commented Jan 22, 2021

That only one person can contribute to. Madness.

yet again i'm expressing myself badly

a pr can have pr .....just for the record

@PhilETaylor

This comment was marked as abuse.

@todordev
Copy link
Contributor

todordev commented Jan 22, 2021

Please, be patient. Let George to take a breath. Everything will be fine and this brunch will be merged with 4.0-dev soon. Let George and Dimitris do it when they're ready.

@brianteeman
Copy link
Contributor

@wilsonge your doing too much. No one can review what you're doing and you don't need to do it on your own. plus your actually holding up others who need to make changes. Just merge this NOW and then everyone else can test, fix and find missing bits.

@brianteeman
Copy link
Contributor

plus i can see silly little errors creeping in because no one can test it etc

@brianteeman
Copy link
Contributor

oh and visually-hidden is not a direct replacement for sr-only so they all need checking :(

@wilsonge
Copy link
Contributor Author

plus i can see silly little errors creeping in because no one can test it etc

Sorry I was sitting on a call with @webgras and @softforge trying to fix issues as they came across them. I've tried to fix as possible

oh and visually-hidden is not a direct replacement for sr-only so they all need checking :(

How come? I think in BS4 that is true but in BS5 i thought it was a straight class rename Renamed .sr-only and .sr-only-focusable to .visually-hidden and .visually-hidden-focusable https://getbootstrap.com/docs/5.0/migration/#helpers As far as I could tell from a quick comparison that was the case too.

@wilsonge wilsonge marked this pull request as ready for review January 22, 2021 21:45
@wilsonge wilsonge merged commit 5785177 into joomla:4.0-dev Jan 22, 2021
@wilsonge wilsonge deleted the bs5 branch January 22, 2021 21:47
@wilsonge wilsonge added this to the Joomla 4.0 milestone Jan 22, 2021
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.