It has become quite clear that managing the "overrides" is rather complicated and bloats this project unnecessarily.

Also, after implementing #2831589: Add grunt-contrib-csslint to maintainer workflow, the "linting" really only catches the LESS source files, not the SASS source files.

It would be better if this lived as a separate project that can be added via JsDelivr CDN or downloaded for source compilation.

Work left todo:

  • Remove all styles
  • Remove all grunt/automation
  • Remove all starterkits
  • Implement this in the base theme's CDN Provider API
  • Update documentation to reflect this change
  • Update change record to further document/discuss this change
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

markcarver created an issue. See original summary.

markhalliwell’s picture

Version: 8.x-3.x-dev » 8.x-4.x-dev

This probably won't happen until 8.x-4.x.

temkin’s picture

Hi @markcarver. First of all, thank you for a great theme! It would be awesome to add a support for Bootstrap 4 to it though. So I started to look at issues that you identified as blockers and got here.

Before I dive deeper I wanted to clarify why you want to move overrides and generated CSS into a separate module, instead of declaring them as a library dependency. At a first glance that could be a better solution, but maybe I'm missing something.

markhalliwell’s picture

Version: 8.x-4.x-dev » 8.x-3.x-dev
Priority: Major » Normal
Status: Active » Needs work
Related issues: +#3031415: Overhaul CDN Providers API

This is now a completely separate repository for all permutations of Drupal Bootstrap styles: https://github.com/unicorn-fail/drupal-bootstrap-styles

It's also published as an NPM package: https://www.npmjs.com/package/@unicorn-fail/drupal-bootstrap-styles

This is to leverage jsDelivr's API/CDN. This new project compiles it's own "api.json" file for the sole purpose of simplifying the massive permutations:

https://cdn.jsdelivr.net/npm/@unicorn-fail/drupal-bootstrap-styles/dist/...

This file is necessary as not all permutations are actually published (there's a ton of duplication, 600+ files at one count).

Instead, there is a symlink key that points to the most recent file that matches its own file contents, e.g.:

{
  name: "/dist/3.4.1/8.x-3.x/drupal-bootstrap.css",
  size: 13387,
  sha512: "RUOeVmsTRyTAbl6b8I+bazmSEhlTmS2Dt58NMpOvED7LJ6qy4CnfSoVoHcQw4a1uFYNxzaeDUbD7lQ/7Vo7OTg==",
  time: "2019-03-25T11:08:07.088Z",
  symlink: "/dist/3.4.0/8.x-3.x/drupal-bootstrap.css"
}

As you can see, https://cdn.jsdelivr.net/npm/@unicorn-fail/drupal-bootstrap-styles/dist/... does not actually exist.

Instead, we'll need to use the symlink value when populating the CDN Provider data in the base theme:

https://cdn.jsdelivr.net/npm/@unicorn-fail/drupal-bootstrap-styles/dist/...

There are, of course, minified equivalents as well:

https://cdn.jsdelivr.net/npm/@unicorn-fail/drupal-bootstrap-styles/dist/...

---

Now that the "bulk" of this work is done, if people want to help tackle the above @todo's, please assign this issue to yourself so I know you're working on it. Also, feel free to ask questions here if you don't understand something.

  • markcarver committed 36dde42 on 8.x-3.x
    Issue #2852156 by markcarver: Move "overrides" source files and...

  • markcarver committed 0cf012e on 8.x-3.x
    Removed starterkits and remaining grunt scripts
    
    Issue #2852156 by...
markhalliwell’s picture

markhalliwell’s picture

zenimagine’s picture

Issue summary: View changes

I do not understand what to do. Currently I have the file locally in my theme and I add to bootstrap_subtheme_front_office.libraries.yml

bootstrap:
  version: 3.4.1
  js:
    bootstrap/js/bootstrap.min.js: {}
  css:
    component:
      bootstrap/css/bootstrap.min.css: {}
      ../../contrib/bootstrap/css/3.4.1/overrides-cosmo.min.css: {}
fontawesome:
  version: 5.7.2
  js:
    fonts/fontawesome/js/all.min.js: {}
global:
  js:
    js/bootstrap.js: {}
    js/back-to-top.js: {}
  css:
    component:
      css/style.css: {}

How can I continue to use the file locally ?

zenimagine’s picture

markhalliwell’s picture

@zenimagine, you can no longer do what you're doing because this project no longer bundles the "overrides" CSS.

The source files have moved to https://github.com/unicorn-fail/drupal-bootstrap-styles, but the compiled/distributed code is only bundled when published as an NPM package.

Instead, you should just add https://cdn.jsdelivr.net/npm/@unicorn-fail/drupal-bootstrap-styles/dist/... to your libraries file as an external resource.

If you really want to incorporate it locally though (not recommended as then it could become stale), just add it to your sub-theme.

markhalliwell’s picture

Issue summary: View changes
markhalliwell’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
20.2 KB

  • markcarver committed e8516c4 on 8.x-3.x
    Issue #2852156 by markcarver: Move "overrides" source files and...
markhalliwell’s picture

This should now automatically inject the external Drupal Bootstrap Styles when using a CDN.

Note, as mentioned above not all permutations are actually published. This means that the file actually used may have a different version, branch or even theme (or lack thereof). While it may be confusing at first, this just means that the file wanted was technically the same as the one it was symlinked to and ultimately served.

While it'd be nice to (semantically at least) have a single file per branch, version, and theme... the reality is that doing so wouldn't be very efficient, especially for a CDN; 747 files totaling ~10 Mb worth of data reduced down to just 134 files and totaling ~2 Mb.

  • markcarver committed 70ab07f on 8.x-3.x
    Update documentation and add new generic starterkit
    
    Issue #2852156 by...
markhalliwell’s picture

Issue summary: View changes
temkin’s picture

So excited to see things moving here! Thanks so much, @markcarver!

I'll be starting a new sub-theme based on this within the next couple of weeks. Although documentation still suggests to use Bootstrap 3.x, I assume that 4.x should be fine as well, right?

I'll give it a try and report here (or in separate issues) if there are any problems.

waverate’s picture

Just providing feedback to the commit at #16.

A. Using CDN Provider

I installed 3.x-dev and followed the directions in /docs/Sub-Theming.md Using the Starterkit. No issues, worked perfectly for using a CDN Provider like jsDelivr.

B. Compiled SASS

I installed 3.x-dev and followed the directions in /docs/Sub-Theming.md Using the Using Source Files SASS. I used a different workflow than Mark's recommendations as it was closer to what I am used to. After creating the subtheme similar to Using the Starterkit, I:

  1. edited /config/install/THEMENAME.settings.yml and uncommented cdn_provider: ''
  2. copied twbs/bootstrap-sass to the theme and renamed it bootstrap. Actually in my case, I have used composer require twbs/bootstrap to download the library and symlinked vendor/twbs/bootstrap-sass to bootstrap in my sub-theme.
  3. copied drupal-bootstrap-styles/tree/master/src/3.x.x/8.x-3.x/scss. In my case, I downloaded and copied the directory to my sub-theme. I'll probably end up loading it as a composer library in the future as it does not have a composer.json file.
  4. compiled scss/styles.scss to css/styles.css. In my case, I loaded composer global require leafto/scssphp and then my executable from the scss directory was pscss < style.scss > ../css/style.css
  5. modified both styles.scss and _default_variables.scss and the changed files compiled correctly

Aside from my own poor workflow, because I don't use front-end tools, the last commit at #12 for the starterkit and the documentation works well.

Thank you Mark.

markhalliwell’s picture

Although documentation still suggests to use Bootstrap 3.x, I assume that 4.x should be fine as well, right?

No, this project has not yet opened up development on 8.x-4.x yet, see
#2554199: Bootstrap 4.

markhalliwell’s picture

copied drupal-bootstrap-styles/tree/master/src/3.x.x/8.x-3.x/scss. In my case, I downloaded and copied the directory to my sub-theme. I'll probably end up loading it as a composer library in the future as it does not have a composer.json file.

No, and it likely won't have one. Composer is meant for PHP libraries, not front-end assets (despite there being several, including a bootstrap one). Composer is also managed at a site-wide/DOCROOT level, meaning the assets would be loaded outside the theme. This is not considered "Best Practice™", especially for multi-site/multi-sub-themes.

All theme assets should be included in the theme itself.

If you really want to use a package manager, you should use npm instead as it's published here https://www.npmjs.com/package/@unicorn-fail/drupal-bootstrap-styles and is a more appropriate method of local installation:

npm i @unicorn-fail/drupal-bootstrap-styles

Then the styles would be located in ./themes/THEMENAME/node_modules/@unicorn-fail/drupal-bootstrap-styles.

As stated in the documentation though, this really falls outside the scope of this project. There are too many ways to do this... how you get it to work or where it's installed is really irrelevant to the steps. The only reason I'm saying anything about this is that there are no plans (and likely never will be any) to include composer support for this external project.

yas’s picture

This is my feedback to the commit at #16

As well as #19 by @waverate, I installed 3.x-dev and followed the directions in /docs/Sub-Theming.md. I also used Starterkit. I could use my sub-theme with using a CDN Provider, jsDelivr. The following is my automated update bash script:

edit: Moved script to https://pastebin.com/VTcq73mZ

markhalliwell’s picture

Re: #19 & #21, while I appreciate the feedback that the steps are working, posting specific workflows is not the goal of this issue/project. That is why the documentation is generic and left up to you to implement however you see fit.

I ask that feedback be limited to actual issues with the documentation, not custom workflows/scripts which detract from the issue at hand.

markhalliwell’s picture

Issue summary: View changes

Created this issue's own dedicated change record: https://www.drupal.org/node/3049485

If someone wants to help tackle this, please feel free to edit that node.

  • markcarver committed ebf1c03 on 7.x-3.x
    Issue #2852156 by markcarver: Move "overrides" source files and...

  • markcarver committed 5a6771e on 7.x-3.x
    Replace existing starterktis with a new generic one and update...
  • markcarver committed bdb7bd8 on 7.x-3.x
    Remove grunt automation and compiled CSS overrides
    
    Issue #2852156 by...
markhalliwell’s picture

Status: Needs review » Fixed
Issue tags: -Needs change record updates, -needs backport to 7.x-3.x

  • markcarver committed 0cf012e on 8.x-4.x
    Removed starterkits and remaining grunt scripts
    
    Issue #2852156 by...
  • markcarver committed 36dde42 on 8.x-4.x
    Issue #2852156 by markcarver: Move "overrides" source files and...
  • markcarver committed 70ab07f on 8.x-4.x
    Update documentation and add new generic starterkit
    
    Issue #2852156 by...
  • markcarver committed e8516c4 on 8.x-4.x
    Issue #2852156 by markcarver: Move "overrides" source files and...

Status: Fixed » Closed (fixed)

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