Upgrade/8525 j query3.6 bootstrap4.6 #8531
Conversation
float:left was removed from bootstrap 4.x col-* rule. now moved into Layouts styles
… conflict with bootstrap grid system used in admin Fixes html classes in order to work with bootstrap 4.x
…hub.com/OrchardCMS/Orchard into Upgrade/8525_JQuery3.6-Bootstrap4.6
…t can know the type of its container and behave accordingly
Removes Bootstrap from Gulp pipeline cause it does not support js modules
…modules so why bootstrap transpilation has been removed from gulp pipeline. The original bootstrap distribution files have been committed also.
…t one was removed in jquery 3
…getBody() function return null, somehow
…JQuery3.6-Bootstrap4.6 # Conflicts: # src/Orchard/UI/Resources/ResourceDefinition.cs
| @@ -237,7 +237,7 @@ function buildJsPipeline(assetGroup, doConcat, doRebuild) { | |||
| )) | |||
| .pipe(plumber()) | |||
| .pipe(gulpif(generateSourceMaps, sourcemaps.init())) | |||
| .pipe(typescript(typeScriptOptions)) | |||
| .pipe(typescript(typeScriptOptions)) // typescript does not support javascript modules | |||
There was a problem hiding this comment.
Typescript should support javascript modules out of the box. I would need to see an example of what you are refering to. An alternative is to build the modules assets with whatever works and to use the gulp pipeline to copy over the bundled assets. If you want to automate that build another option is to call for example Webpack from Gulp.
There was a problem hiding this comment.
I am talking specifically about gulp-typescript here, because I'm not familiar enough with typescript to make blanket statements.
Typescript does support javascript modules, as long as there also are other dependencies. I mean to say that the transpilation step for modules that are written in .js files results in javascript that depends on other libraries.
For example, it could create javascript that depends on system.js, or amd.js, but doesn't seem to be able to produce javascript that stands on its own. The specific library the js will depend on is "selected" by a property in typeScriptOptions, but the library itself is not automatically included (it would be hard to do so at this stage), so the end result is that the resulting js fails to execute.
There was a problem hiding this comment.
Have you tried updating gulp-typescript?
"gulp-typescript": "3.1.4",
Because as you can see here :
We are using a newer version. And also, when you say that it produces a non-working javascript file maybe that's because we were back then using a lower version of TypeScript too. So, one thing that could be done here is to try updating TypeScript to the latest version and try to transpile manually these modules assets. Then, if it works, update the gulp-typescript version to the latest version too, and see if that works.
We would need to update the Documentation about the Gulp pipeline so that we know which exact version everything was built with including the TypeScript version used on the system it was built with.
There was a problem hiding this comment.
Basically it should support TypeScript 2.2.0
| @@ -26,9 +26,6 @@ | |||
| Style.Include("LayoutEditor.css", "LayoutEditor.min.css"); | |||
| Script.Include("LayoutDesignerHost.js"); | |||
|
|
|||
| // The grid system. | |||
| Style.Include("default-grid.css"); | |||
There was a problem hiding this comment.
This removes probably extensibility.
There was a problem hiding this comment.
Is that related to the Parent: Row thing? People use it on the front-end for custom themes.
There was a problem hiding this comment.
This is only in the editor shape for layouts.
I think for front-end you are referring to what's in TheThemeMachine theme? That's a separate css file (with the same name and perhaps same content) that is included in the layout there.
Here in the backoffice editor for the layout, the content of that stylesheet "conflicts" with bootstrap's grid system. I would argue that the stylesheet itself should also be removed from Orchard.Layouts.
In case someone would want to extend styling for this editor, as it is, they would have to go in the sources and change that stylesheet, and at that point one might as well add a Style.Include("my-new.css"); line in LayoutEditor.cshtml.
There was a problem hiding this comment.
Should not be removed from Orchard.Layouts, because that's the default to use when it's not themed. If there is a conflict on the admin, then we need to ensure that all existing classes are also in bootstrap. Or change the names of the classes to they don't collide.
|
It sucks that we left this PR here, but what do you think about upgrading to 3.7 now too? |
|
I comment on this PR in order to resume it and try to finalize the work. |
|
I'd prefer to merge it with the affected libraries updated to the latest stable version (I can help with that). We'll also need some time to test it in a client project (although that's already using jQuery 3.6 for custom components). But this really is a much-needed update! |
|
@BenedekFarkas can we plan a dedicated meeting to bring this PR to 1.10.x or dev? In particular we are not able to make the gulp pipeline working properly see #8525. |
Yes, sounds good! Sometime next week works for me, I'll check out the corresponding issue(s)/PR(s) and get back to you! |
|
@BenedekFarkas I'll ping you on Gitter so we can sync for a call |
|
Sorry, I don't really use Gitter - BTW on the topic of communications, if you and/or your team use Discord, then I'd prefer that, and we can set up a channel there just for preparing for 1.11. |
Hi @BenedekFarkas I can propose you to join us on Discord next tuesday at 11AM Italian time. |
Upgrade to jQuery 3.6, jQueryUI 1.13 and Boostrap 4.6.1
From our tests everything seems to work correctly, if someone can take a look into this, they are welcome.
We will continue to test it during next days anyway