SHOR-109: Manage bootstrap-sass via npm, upgrade to v3.4

[AkA84]

The main goal of this PR is to make Shoreditch upgrade from Bootstrap v3.3.5 to v3.4, which contains two important XSS fixes: twbs/bootstrap@29f9237, twbs/bootstrap@13bf8ae

Given that the dependency on Bootstrap (more specifically, on bootstrap-sass) was not being managed gracefully (the package's source code had been simply copy-pasted in the base/ folder), the PR also improves things on that front by properly declaring bootstrap-sass as a devDependency in the package.json file

"devDependencies": {
  "bootstrap-sass": "<version>"
}

Note: switch to bootstrap-sass as npm package

When switching to using bootstrap-sass as an npm package, the version used first was v3.3.5 (the same version that was manually copy-pasted in the base/ folder), before upgrading to v3.4

This was to make sure that no changes would result in the minified css files when running gulp sass after the switch, and before the upgrade

Note: changes introduced by the new releases

The changes introduced by v3.3.6, v3.3.7 and v3.4 had been checked both manually on the styleguide and by running BackstopJS on the styleguide extension, on core screens, on CiviHR, and on Mosaico

No unexpected changes were detected

Note: v3.4 release

At the time of this writing, the v3.4 release is currently on hold, aiming for a ~December 10th release date

Given that, bootstrap-sass doesn't have a new release either, so the alternative is to simply point the dependency to the next branch until v3.4 will be properly released

"devDependencies": {
  "bootstrap-sass": "twbs/bootstrap-sass#next"
}

Technical details

Removal of bootswatch themes

One feature we were considering during the early stage of development was to give the option to a user/dev to switch between the default Bootstrap style and one of the many Bootswatch themes

That was the reason for having a base/scss/themes/ folder, where the user could add the Bootswatch theme he wanted, and this line in base/scss/bootstrap.scss, where the user could switch the currently enabled theme

// Or "themes/superhero", etc
@import "themes/default";

In the end this didn't really go anywhere, but still we were left with this awkward set up that no one was using

This PR clean up and simplifies things, by removing the whole base/scss/themes folder, and placing in base/scss/bootstrap.scss the default Bootstrap's @imports list (which was before in base/scss/themes/_default.scss)

Move custom partials outside of base/vendor/

In the base/scss/bootstrap.scss file one should find only the aforementioned default @imports list (see here for example), but instead we had a handful of custom @imports that shouldn't have been placed in what is in effect a vendor file

@import "overrides/variables";        // <--
@import "mixins/initial-value";       // <--

// Core variables and mixins
@import "vendor/bootstrap/variables";
@import "vendor/bootstrap/mixins";

// Reset CMS and old CiviCRM style
@import "overrides/reset/drupal";     // <--
@import "overrides/reset/civicrm";    // <--

// Core CSS
@import "bootstrap/scaffolding";
@import "bootstrap/type";
// Rest of default @imports follows here..

The custom partials had now been moved, the end result being that base/scss/ now contains only the original bootstrap-sass source code and nothing else

Before

scss/
├── mixins/
│   └── initial-value.scss
├── overrides/
│   ├── reset/
│   │   ├── _civicrm.scss
│   │   └── _drupal.scss
│   └── _variables.scss
├── themes/
│   └── // Bootswatch partials
└── vendor/
    ├── bootstrap/
    │   └── // bootstrap-sass partials
    └── bootstrap.scss

After

scss/
└── vendor/
    ├── bootstrap/
    │   └── // bootstrap-sass partials
    └── bootstrap.scss

Following are details about each of the partials that have been moved:

overrides/variables

$bootstrap-namespace : "#bootstrap-theme";

$icon-font-path: "../fonts/" !default;

$crm-wizard-active-bg-color: $gray-lighter;
$crm-wizard-active-font-color: $brand-primary;

This file was removed and its content placed in scss/bootstrap/overrides/_variables.scss, where all the other override/custom variables reside

Couple of notes:

• The $icon-font-path variable was not moved and thus simply ignored, as the "../fonts/" value was never applied given that scss/bootstrap/overrides/_variables.scss was already defining a value for it, which was the one eventually used

  $icon-font-path: '../base/fonts/' !default;

• The $crm-wizard-* variables had been marked with !default, so that their value can be overridden if needed

  //== CRM Wizard
  //
  //##
  
  $crm-wizard-active-bg-color: $gray-lighter !default;
  $crm-wizard-active-font-color: $brand-primary !default;

mixins/initial-value

This mixin had been placed in the scss/bootstrap/mixins along with all the other mixins

overrides/reset/* partials

These partials had been placed in the newly created scss/bootstrap/reset folder, and @imported right before the default Bootstrap styles (just as it was happening before)

// scss/bootstrap/bootstrap.scss

// Reset CMS and old CiviCRM style before adding Bootstrap styles
@import 'bootstrap/reset/drupal';
@import 'bootstrap/reset/civicrm';

@import 'base/scss/bootstrap';

Each reset partial internally @imports the original Bootstrap variables and mixins in order to be properly processed

// scss/bootstrap/reset/_drupal.scss

@import 'base/scss/vendor/bootstrap/variables';
@import 'base/scss/vendor/bootstrap/mixins';

Fix vars declaration

The $text-muted variable was being overridden incorrectly: instead of assigning a new value in overrides/_variables.scss, the new value was being assigned directly on the original _variables.scss partial (see here)

The $icon-font-path variable was instead being declared, first here

$icon-font-path: '../base/fonts/' !default;

then here

$icon-font-path: '../vendor/bootstrap/fonts/bootstrap/' !default;

Given the usage of !default, the second declaration was always ignored, so there was no point keeping it around (the fist declaration was the correct one anyway)

Add gulp copy tasks

Given that the bootstrap-sass package is placed in the node_modules/ folder when installed, a group of copy:xyz gulp tasks had been created in order to copy fonts, scripts, and scss partials in the base/ folder

const ASSETS_PATH = path.join(__dirname, '/node_modules/bootstrap-sass/assets/');

gulp.task('copy:fonts', () => {
  const source = path.join(ASSETS_PATH, 'fonts/bootstrap/*');

  return gulp.src(source).pipe(gulp.dest('base/fonts'));
});

gulp.task('copy:js', () => {
  const source = path.join(ASSETS_PATH, 'javascripts/bootstrap/*');

  return gulp.src(source).pipe(gulp.dest('base/js'));
});

gulp.task('copy:sass', () => {
  const source = path.join(ASSETS_PATH, 'stylesheets/bootstrap/**/*');

  return gulp.src(source).pipe(gulp.dest('base/scss/vendor/bootstrap'));
});

gulp.task('copy', gulp.parallel('copy:sass', 'copy:js', 'copy:fonts'));

the copy task is then being called automatically on the postinstall hook

"scripts": {
  "postinstall": "npx gulp copy && npx gulp sass"
}

Note

A better set up would be to have only the fonts and scripts copied over, while keeping the sass partials in node_modules, which would then be @imported automatically when running gulp sass

var includePaths = [
  __dirname,
  path.join(__dirname, 'node_modules'),
  path.join(__dirname, 'scss')
];

// ...
gulp.task('sass:bootstrap', function () {
  gulp.src('scss/bootstrap/bootstrap.scss')
    .pipe(bulk({
      includePaths: includePaths
    }))
    .pipe(sass({
      outputStyle: 'compressed',
      includePaths: includePaths,
      precision: 10
    }).on('error', sass.logError))
});

// scss/bootstrap/bootstrap.scss

@import 'bootstrap-sass/assets/stylesheets/bootstrap';

This would eliminate the need for having a base/scss folder at all.

Despite that, and in order to keep the scope of this PR limited and to avoid changes in downstream projects (there are a handful that are @importing partials in base/scss/vendor directly), this set up was scrapped for a more "conservative" approach (= keep the folder structure as-is)

Fixed permissions on base/ files

The vast majority of the base/ files had the permissions set to 755. This was fixed automatically when copying over the files from the bootstrap-sass package, and now the permission are set to a more restrictive 644

Ignore linting on base/ files

Both stylelint and semistandard are now configured to ignore any files in base/ (given that it needs to be considered a vendor folder)

"semistandard": {
  "ignore": [
    "base/js/*"
  ]
}

// .stylelintignore

base/scss/

[igorpavlov-zz]

Please provide a bit more extended explanation which files are copied, where and why.

[igorpavlov-zz]

const ASSETS_PATH = path.join(__dirname, 'node_modules/bootstrap-sass/assets');

No trailing slashes are needed when using path join.

console.log(path.join('/Users', '/igorpavlov', 'www/', '/site/', 'index.html')) will output /Users/igorpavlov/www/site/index.html.

[igorpavlov-zz]

You have 2 blocks with the similar code, please consider creating a function:

gulp.task('copy:js', () => {
  return copyFiles('javascripts/bootstrap/*', 'base/js')
});

function copyFiles (source, destination) {
  const sourceAbsolute = path.join(ASSETS_PATH, source);

  return gulp.src(sourceAbsolute).pipe(gulp.dest(destination));
}

[igorpavlov-zz]

please remove the trailing '/'

path.join(__dirname, 'node_modules/bootstrap-sass/assets', source)

[igorpavlov-zz]

Please align the params with return:

* @param  {String} source
* @param  {String} dest
* @return {Object}