RFC: Force-Load Bootstrap should be no-go #566

iJungleboy · 2020-05-30T08:39:32Z

I'm creating a clean, hopefully best-practice setup for themes https://github.com/2sic/oqtane-themes-template

But now I ran into an issue which is a no-go from a designers point of view: My setup uses SASS to build a custom CSS bootstrap build. That's the pro-way of doing this, because you can fine tune what CSS you include, what you customize etc. In my sample this would happen here: https://github.com/2sic/oqtane-themes-template/blob/master/Src/theme.scss

To my surprise I noticed that even though I have everything in my CSS, a second bootstrap was being loaded at the same time:

Issues with this setup

This is really, really bad from a designers point of view. Here's why:

If I don't want bootstrap - or maybe just not bootstrap 4 (because 5 is on the way) I can't escape it
If I do use bootstrap as a pro, creating my own bundle, I may run into issues
i'm just loading too many css rules which I don't want (performance)
When a certain version of bootstrap has a security issue, then things get really messy
The combination of too many rules, including very similar ones, increases the testing/debugging effort across devices massively

I believe the reason bootstrap is pre-included is actually because of the admin ui, so I think #565 is even more important.

Goals

My understanding of goals ATM is this:

Initial setups should look great
Admin UI should look good and work flawlessly

I would like to add these goals now:

Admin UI should look good and work flawlessly all the time (see RFC: Move all Admin features away from Theming influences. #565)
The designer should have 100% control of output - html, js, css at all times. Oqtane should not restrict or constrict in any way.
The designer must also have 100% control of icons etc. loaded - these can easily cost 20 page-speed points! not kidding

Steps IMHO

First make sure that admin-UI is completely Independent of the normal pages. This will make lots of future stuff much easier, no side-effects, less testing, etc. - RFC: Move all Admin features away from Theming influences. #565
Then determine a very minimum of classes which are used in in-page editing. These should of course only be included in edit-mode, and never otherwise. So a runtime-page should have ZERO CSS or icons for edit mode
In-page Icons used for editing should be SVGs only - ideally bundled inside the JS or CSS that uses them (performance). This is easy to do, I can help get this up and running.
Then the _hosts must remove all references to bootstrap.css, app.css, jquery.js, popper.js, bootstrap.js etc.

This I find critically important - and something DNN failed to do early, which is why it's so hard to create stuff on DNN that performs really well - you always have to patch around stuff in DNN.

What do you all think? Votes, comments, thoughts?

The text was updated successfully, but these errors were encountered:

mikecasas · 2020-05-30T09:09:19Z

I'm getting 404s on your https://github.com/2sic/oqtane-themes-template

I do like your suggestion of #565. I created a theme, and then when I went into the admin pages my own theme made it a mess.

I think the default bootstrap theme in the _host file was just a fall back, but it caught me too when my site colors changed...I couldn't figure out where it was coming from.

iJungleboy · 2020-05-31T07:29:03Z

@mikecasas repo was private, fixed now ;)

The hosts is not a fallback, it's hardwired. ATM 6 ressources (2 css, 3 js) are always loaded (in addition to 2 JS which are probably necessary for blazor, interop and blazor....js)

...
<head>
...
    <link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.3.1/css/bootstrap.min.css" integrity="sha384-ggOyR0iXCbMQv3Xipma34MD+dH/1fQ784/j6cY/iJTQUOhcWr7x9JvoRxT2MZw1T" crossorigin="anonymous">
    <link rel="stylesheet" href="css/app.css" />
</head>
<body>
...
    <script src="js/interop.js"></script>
    <script src="https://code.jquery.com/jquery-3.3.1.slim.min.js" integrity="sha384-q8i/X+965DzO0rT7abK41JStQIAqVgRVzpbzo5smXKp4YfRvH+8abtTE1Pi6jizo" crossorigin="anonymous"></script>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.14.7/umd/popper.min.js" integrity="sha384-UO2eT0CpHqdSJQ6hJty5KVphtPhzWj9WO1clHTMGa3JDZwrnQq4sF86dIHNDz0W1" crossorigin="anonymous"></script>
    <script src="https://stackpath.bootstrapcdn.com/bootstrap/4.3.1/js/bootstrap.min.js" integrity="sha384-JjSmVgyd0p3pXB1rRibZUAYoIIy6OrQ6VrjIEaFf/nJGzIxFDsf4x0xIM+B07jRM" crossorigin="anonymous"></script>

    @if (Configuration.GetSection("Runtime").Value == "WebAssembly")
    {
        <script src="_framework/blazor.webassembly.js"></script>
    }
    else
    {
        <script src="_framework/blazor.server.js"></script>
    }
</body>
</html>

mikecasas · 2020-05-31T11:15:24Z

I guess I meant fallback as opposed to nothing. Your repo looks good. A lot to chew on.

sbwalker · 2020-05-31T14:55:17Z

Philosophically Oqtane is a module application framework and in order to create a predictable development experience ( which is a significant problem in DNN ) it must have a consistent base UX framework which developers can use in their modules. Bootstrap was chosen as the base UX framework because of its maturity and adoption, and because it covers the majority of UX scenarios a module developer wil encounter. It should be noted that because Oqtane is not a CMS, the emphasis is on consistency for the developer rather than control/flexibility for the designer. It should also be noted that there is no "Admin UI" - the default admin features are simply a bunch of modules that are included by default in a site.

From a technical perspective, Oqtane is a single page application. As a result there are no full page refreshes after the initial page load ( in both Blazor Server and Blazor WebAssembly ). As a result, JavaScript and CSS references need to be added to the DOM dynamically. There are some issues in getting this to work consistently when running in Blazor ( see #518 ). Therefore the essential framework resources are included in _host.cshtml ( server-side ) for fallback purposes. If the dynamic approach was 100% reliable then maybe these resources could be moved from _host.cshtml to the themes - however I am not sure if this even makes sense given that the goals are to provide developers with a consistent UX framework to target.

thabaum · 2020-05-31T15:40:02Z

CSS classes should be what controls styles in Oqtane

We had a discussion about this I was going to comment on this before just not sure how to put it in a way I believe it is to be done.

app- is the class for the app, now app-admin- or app-dashboard- classes need added. just something generic that describes each part.

In your theme you will need to select things and adjust the dashboard as well to fit the theme if you are editing bootstrap theme directly... which is what it sounds like you guys are doing. Make your own classes and edit those ONLY or deal with editing the dashboard as well to adjust as needed.

Dashboard is not responsive... i put a PR to put things in DIVs at least and then was going to allow adding classes more easily for each page. That can be a continued discussion where you can test that UI and submit some suggestions or comments if not a PR.

I am also trying to work on the notifications area... just too much to take on all the wishes for each area at once. It can go on forever. And we may just take parts of my PR's and gut the entire dashboard UI.

Same thing with modules and the same when creating themes. You should not need to load another bootstrap. If anything we need a UI to change the version to the latest... or an update feature that keeps it current checking for the latest version whenever oqtane gets updated.

This keeps things current with bugs and security fixes.

Also a new understanding of how to design themes for Oqtane will need structured and developed. I believe we need to look at Blazorise for examples of things we can incorporate directly into the project so you can create themes using tools very rapidly, professionally and in a fun way. The sky is the limit here in what can be done. It is just what can we agree to do that makes best logical sense for Oqtane's objective.

thabaum · 2020-05-31T15:45:27Z

Can we use something like Fluent UI https://developer.microsoft.com/en-us/fluentui#/ ?

sbwalker · 2020-05-31T16:49:23Z

I will keep referring to the Oqtane Philosophy blog which was published so that people understand the goals and approach:

https://www.oqtane.org/Resources/Blog/PostId/538/oqtane-philosophy

See the section for Managed Dependencies. We will not be adding dependencies to the core framework unless they are absolutely critical.

sbwalker · 2020-05-31T16:50:54Z

I believe app- classes were added to all theme components already so that styles can be specifically targeted.

sbwalker · 2020-05-31T16:52:23Z

And there is already an update service in the framework to try and simplify the effort in upgrading an installation

iJungleboy · 2020-06-02T06:45:32Z

@sbwalker I understand the thinking that as an application framework, you want to make sure that some parts are already there. My suggestion is to do this consistently at a different level: the theme.

Reason 1: Everything will become Obsolete

No matter how good your current choice of CSS-framework may be (we're also in for BS4), in a year or two it will be wrong. So if in 2 years And then you you either end up producing a breaking change across all installations unless they stop upgrading.

Or in you let the community work out what they want, and switch when they want - by placing this responsibility in the theme.

Reason 2: Other people have different Quality-Expectations

We work with BS4 every day and fine-tunes what it should do. For that, the current implementation is wrong, because the preloaded version causes problems I can't fix in a clean way. And depending on the project, something different may be wrong - so there is no 1 right.

Moving it completely to the theme let's people pick their own poison.

Alternative: Let themes Opt-Out

If the pre-build includes feel very important to you, I suggest that a theme could opt-out of these. By supporting an interface, hosting an attribute etc. Or it could also be at the config level. But IMHO the theme would be better for this, as it allows distribution (which config-changes don't really).

PavelVsl · 2020-06-02T16:33:33Z

The only reason BS must be in _Host.cshtml is the dependency of the administrative interface.
I think the way is to separate the look of the user interface from the data model. This will allow the creation of interface skins for the required technologies while maintaining security parameters.
Divide Oqtane.Client into body and variable head. Create a headless layer of base classes. Forms from the head will use base classes from body. Administrative interface "skin" can be part of theme.

sbwalker · 2020-06-02T17:51:48Z

Oqtane is a modular framework and one of its primary goals is to allow people to assemble a solution from a variety of modules from different sources. So lets consider a scenario where Developer1 created ModuleA, Developer2 created ModuleB, and Developer3 created ModuleC... and I am building a solution which is going to incorporate all of these modules. I can install these modules and I would expect that they would all share a consistent look and feel so that the overall solution has a seamless user experience. The only way to accomplish this is by having a common default design system which all modules utilize. Bootstrap is the common design system in Oqtane to ensure this consistency. And the current Admin modules use Bootstrap for this reason - so they can be a seamless part of a fully integrated solution.

However there is also a scenario where someone has no intention of ever using any modules except for ones they create themselves. This is not the main use case which Oqtane was designed for but it is still valid. In this case, the developer could choose to use a totally different design system in their modules. The only challenge they will encounter in going down this path is that the Admin modules may not integrate seamlessly from a visual perspective. This would not be Oqtane's responsibility at this point, it would be up to the developer to deal with, and there are a variety of possible solutions they could explore.

sbwalker · 2020-06-02T18:09:43Z

Back to the default loading of Bootstap topic. I agree that it should be moved out of _host,cshtml and be explicitly registered in the theme like all of the other resources currently. However as I explained earlier in this thread, there are some technical issues related to the dynamic loading of resources. The dynamic loading of stylesheets seems to work fine but the dynamic loading of scripts which have dependencies on other scripts is currently not 100% reliable ( see #518 ).

iJungleboy · 2020-06-02T18:17:12Z

@sbwalker awesome 🚀. I'll help where I can.

Just fyi: i already created a theme-template for general themes and will now create one specifically for bootstrap4 with proper SASS style generation etc.

sbwalker · 2020-06-02T20:13:10Z

I just experimented with moving the _host.cshtml resource declarations to the theme files. It works fine for stylesheet references, but not for Javascript references. The problem is that there are dependencies between the JQuery, Popup, and Bootstrap JS files - which is the same problem experienced with Quill in #518. If the JavaScript references are statically declared in the _host.cshtml file they work fine - but not when they are dynamically added to the page.

iJungleboy · 2020-06-03T06:15:00Z

@sbwalker Awesome for step 1 :)

Perhaps I can help with the second problem, but the thread on #518 is extensive. I'll try to go through it and maybe find something I can help with...

sbwalker · 2020-06-04T15:58:32Z

I have not merged any changes related to migrating resource declarations to themes for a couple of reasons:

The Javascript resource loading is not reliable
I have not figured out how to make this backward compatible. Basically, by removing resources from _host.cshtml and moving them to the theme it means that any existing third party themes will be broken as they are relying on the current behavior.

thabaum · 2020-06-07T00:00:41Z

but not when they are dynamically added to the page.

is there a way to insert some sort of template component that handles getting things to the file tricking it into thinking it is static information?

sbwalker · 2020-06-15T19:42:17Z

@jimspillane I tried again to move the resources out of the _host using the same pattern we used for Quill:

var Oqtane = Oqtane || {};

Oqtane.OqtaneTheme = {
    loadBootstrap: async function () {
		const load = loadjs(['https://code.jquery.com/jquery-3.3.1.slim.min.js', 'https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.14.7/umd/popper.min.js', 'https://stackpath.bootstrapcdn.com/bootstrap/4.3.1/js/bootstrap.min.js'], 'Bootstrap', {
			async: false,
			returnPromise: true,
			before: function (path, element) {
				if (path === 'https://code.jquery.com/jquery-3.3.1.slim.min.js') element.crossOrigin = 'anonymous';
				if (path === 'https://code.jquery.com/jquery-3.3.1.slim.min.js') element.integrity = 'sha384-q8i/X+965DzO0rT7abK41JStQIAqVgRVzpbzo5smXKp4YfRvH+8abtTE1Pi6jizo';
				if (path === 'https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.14.7/umd/popper.min.js') element.crossOrigin = 'anonymous';
				if (path === 'https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.14.7/umd/popper.min.js') element.integrity = 'sha384-UO2eT0CpHqdSJQ6hJty5KVphtPhzWj9WO1clHTMGa3JDZwrnQq4sF86dIHNDz0W1';
				if (path === 'https://stackpath.bootstrapcdn.com/bootstrap/4.3.1/js/bootstrap.min.js') element.crossOrigin = 'anonymous';
				if (path === 'https://stackpath.bootstrapcdn.com/bootstrap/4.3.1/js/bootstrap.min.js') element.integrity = 'sha384-JjSmVgyd0p3pXB1rRibZUAYoIIy6OrQ6VrjIEaFf/nJGzIxFDsf4x0xIM+B07jRM';
			}
		})
        .then(function () { })
        .catch(function (pathsNotFound) { });

		await load;
    }
};

Unfortunately it does not work 100%. Specifically, the tab panel control which is used throughout the admin UI no longer functions correctly. The tab panel control uses standard Bootstrap/jQuery markup but it appears that the event listeners are not attached properly after making this change ( although if you inspect the page source it appears that they are ).

jimspillane · 2020-06-15T20:53:59Z

@sbwalker I will take a look at it tonight.

jimspillane · 2020-06-16T02:00:28Z

Removing the click event listener from blazor.server.js enables navigation between the tabs.

Prevent default actions appears to remedy the issue. In TabStrip.razor try adding @onclick:preventDefault="true" to the links.

<ul class="nav nav-tabs" role="tablist">
                @foreach (TabPanel tabPanel in _tabPanels)
                {
                    <li class="nav-item">
                        @if (tabPanel.Name == ActiveTab)
                        {
                            <a class="nav-link active" data-toggle="tab" href="#@tabPanel.Name" role="tab" @onclick:preventDefault="true"> 
                                @DisplayHeading(tabPanel.Name, tabPanel.Heading)
                            </a>
                        }
                        else
                        {
                            <a class="nav-link" data-toggle="tab" href="#@tabPanel.Name" role="tab"  @onclick:preventDefault="true">
                                @DisplayHeading(tabPanel.Name, tabPanel.Heading)
                            </a>
                        }
                    </li>
                }
</ul>

jimspillane · 2020-06-16T02:19:32Z

It looks like CSS isolation is going to become a reality. This might be something to keep an eye on as we discuss styling.

sbwalker · 2020-06-16T12:16:01Z

Yes I noticed that thread as well... it will be interesting to see how they implement it.

sbwalker · 2020-06-16T12:22:36Z

@jimspillane thank you for investigating the JavaScript issue. I am guessing that if Bootstrap/jQuery are loaded as part of _host.cshtml and they happen BEFORE blazor.server.js is loaded then the event listeners get wired up correctly. However, when you load Bootstrap/jQuery dynamically it occurs AFTER blazor.server.js and as a result the event listeners do not get wired up correctly. Does this sound correct? If so, this could lead to some other obscure issues as well ( ie. I am thinking about third party component libraries which rely on JavaScript, etc... )

jimspillane · 2020-06-16T12:44:34Z

I did try putting the scripts after blazor.server.js in the _host.cshtml file and it worked fine. blazor.server.js might be adding the event listeners after the page load.

    @if (Configuration.GetSection("Runtime").Value == "WebAssembly")
    {
        <script src="_framework/blazor.webassembly.js"></script>
    }
    else
    {
        <script src="_framework/blazor.server.js"></script>
    }

 <script src="js/interop.js"></script>
    <script src="https://code.jquery.com/jquery-3.3.1.slim.min.js" integrity="sha384-q8i/X+965DzO0rT7abK41JStQIAqVgRVzpbzo5smXKp4YfRvH+8abtTE1Pi6jizo" crossorigin="anonymous"></script>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.14.7/umd/popper.min.js" integrity="sha384-UO2eT0CpHqdSJQ6hJty5KVphtPhzWj9WO1clHTMGa3JDZwrnQq4sF86dIHNDz0W1" crossorigin="anonymous"></script>
    <script src="https://stackpath.bootstrapcdn.com/bootstrap/4.3.1/js/bootstrap.min.js" integrity="sha384-JjSmVgyd0p3pXB1rRibZUAYoIIy6OrQ6VrjIEaFf/nJGzIxFDsf4x0xIM+B07jRM" crossorigin="anonymous"></script>

From what I read, the event listeners are not guaranteed to fire in the order they are added, however, most browsers implement it that way. This behavior appears to be a common issue for other use cases too dotnet/aspnetcore#5545

resolve #566 by moving Bootstrap declaration into theme

jimspillane · 2020-06-17T01:34:58Z

@sbwalker the installer lost its style.

sbwalker · 2020-06-17T01:45:46Z

Thank you for letting me know. The install wizard component was relying on Bootstrap in _host.cshtml - although the styling was minimal.

jimspillane · 2020-06-17T01:54:44Z

I added a link to Bootstrap in the Installer.razor file and it works.

<link href="https://stackpath.bootstrapcdn.com/bootstrap/4.5.0/css/bootstrap.min.css" rel="stylesheet">

The other changes tested well ⭐

thabaum mentioned this issue Jun 5, 2020

Style Development: Theme-, Module-, App- wrapped with <div> + class selector for styling conflict issues #584

Closed

sbwalker closed this as completed in 7c24bae Jun 16, 2020

sbwalker added a commit that referenced this issue Jun 16, 2020

Merge pull request #621 from sbwalker/master

d4d12b6

resolve #566 by moving Bootstrap declaration into theme

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC: Force-Load Bootstrap should be no-go #566

RFC: Force-Load Bootstrap should be no-go #566

iJungleboy commented May 30, 2020

mikecasas commented May 30, 2020 •

edited

Loading

iJungleboy commented May 31, 2020

mikecasas commented May 31, 2020

sbwalker commented May 31, 2020 •

edited

Loading

thabaum commented May 31, 2020 •

edited

Loading

thabaum commented May 31, 2020

sbwalker commented May 31, 2020

sbwalker commented May 31, 2020

sbwalker commented May 31, 2020

iJungleboy commented Jun 2, 2020

PavelVsl commented Jun 2, 2020

sbwalker commented Jun 2, 2020

sbwalker commented Jun 2, 2020

iJungleboy commented Jun 2, 2020

sbwalker commented Jun 2, 2020

iJungleboy commented Jun 3, 2020

sbwalker commented Jun 4, 2020

thabaum commented Jun 7, 2020 •

edited

Loading

sbwalker commented Jun 15, 2020

jimspillane commented Jun 15, 2020

jimspillane commented Jun 16, 2020 •

edited

Loading

jimspillane commented Jun 16, 2020

sbwalker commented Jun 16, 2020

sbwalker commented Jun 16, 2020

jimspillane commented Jun 16, 2020

jimspillane commented Jun 17, 2020

sbwalker commented Jun 17, 2020

jimspillane commented Jun 17, 2020 •

edited

Loading

RFC: Force-Load Bootstrap should be no-go #566

RFC: Force-Load Bootstrap should be no-go #566

Comments

iJungleboy commented May 30, 2020

Issues with this setup

Goals

Steps IMHO

mikecasas commented May 30, 2020 • edited Loading

iJungleboy commented May 31, 2020

mikecasas commented May 31, 2020

sbwalker commented May 31, 2020 • edited Loading

thabaum commented May 31, 2020 • edited Loading

thabaum commented May 31, 2020

sbwalker commented May 31, 2020

sbwalker commented May 31, 2020

sbwalker commented May 31, 2020

iJungleboy commented Jun 2, 2020

Reason 1: Everything will become Obsolete

Reason 2: Other people have different Quality-Expectations

Alternative: Let themes Opt-Out

PavelVsl commented Jun 2, 2020

sbwalker commented Jun 2, 2020

sbwalker commented Jun 2, 2020

iJungleboy commented Jun 2, 2020

sbwalker commented Jun 2, 2020

iJungleboy commented Jun 3, 2020

sbwalker commented Jun 4, 2020

thabaum commented Jun 7, 2020 • edited Loading

sbwalker commented Jun 15, 2020

jimspillane commented Jun 15, 2020

jimspillane commented Jun 16, 2020 • edited Loading

jimspillane commented Jun 16, 2020

sbwalker commented Jun 16, 2020

sbwalker commented Jun 16, 2020

jimspillane commented Jun 16, 2020

jimspillane commented Jun 17, 2020

sbwalker commented Jun 17, 2020

jimspillane commented Jun 17, 2020 • edited Loading

mikecasas commented May 30, 2020 •

edited

Loading

sbwalker commented May 31, 2020 •

edited

Loading

thabaum commented May 31, 2020 •

edited

Loading

thabaum commented Jun 7, 2020 •

edited

Loading

jimspillane commented Jun 16, 2020 •

edited

Loading

jimspillane commented Jun 17, 2020 •

edited

Loading