FRONT-2362: Refactor theme and sub-theme to use bcl-builder. #10

papegaill · 2021-07-12T12:56:59Z

Jira issue(s):

FRONT-2362

PR goals:

Remove Laravelmix tasks and use bcl-builder, https://github.com/openeuropa/bootstrap-component-library
Update bootstrap-ie11 to latest v5.0.2
Add components module to create @oe-bcl namespace for bcl twig templates copied to templates/bcl folder

drishu

disclaimer: I haven't double checked all the paths

README.md

composer.json

includes/page_attachments.inc

README.md

kits/default/package.json

kits/default/README.md

papegaill · 2021-07-28T15:10:27Z

Many thanks for the review and updates @drishu I updated the PR accordingly.

docker-compose.yml

kits/default/bcl-builder.config.js

drishu

Working on the whitelabel I noticed one missed point about the disabling of the system style sheets

kits/default/default.info.hidden.yml

papegaill · 2021-08-06T11:58:40Z

Working on the whitelabel I noticed one missed point about the disabling of the system style sheets

Thanks for the review, PR updated accordingly.

…e bootstrap-ie11 to 5.0.1.

…ommand.

…rupal core.

…to .libraries.yml.

…file.

Co-authored-by: Szilagyi Andras <aszilagyi@live.com>

brummbar · 2021-08-09T15:21:37Z

kits/default/default.info.hidden.yml

-base theme: oe_bootstrap_theme
+base theme: OE_BOOTSTRAP_THEME_SUBTHEME_MACHINE_NAME


This needs to stay oe_bootstrap_theme as it's the base theme.

brummbar · 2021-08-09T15:27:49Z

kits/default/default.info.hidden.yml

+  oe_bootstrap_theme/bcl: false
+  oe_bootstrap_theme/bcl: false


Why we remove the bcl from the original theme? We can keep the original coming from oe_bootstrap_theme, we don't even have to include it in the builder.

The idea was to keep the default bcl theme, as for most projects this will just be replaced by another project-specific bcl theme. My intention was thus to keep the config of the builder and to just replace the package .. in order to facilitate the setup for the future projects. but I agree it's confusing! it will undoubtedly be necessary to improve the documentation in a second step..

brummbar · 2021-08-09T15:30:17Z

kits/default/default.libraries.yml

+bcl:
+  js:
+    assets/js/oe-bcl-default.bundle.min.js: {}
+  css:
+    theme:
+      assets/css/oe-bcl-default.min.css: {}
+
+custom_style:


As written above, we can revert this.

As mentioned above, ok to remove the config from the theme by default ..

brummbar · 2021-08-09T15:30:57Z

kits/default/package.json

+    "@openeuropa/bcl-builder": "0.4.0",
+    "@openeuropa/bcl-theme-default": "0.4.0",
+    "bootstrap-ie11": "5.0.2",
+    "chokidar-cli": "1.2.0",
+    "copyfiles": "2.4.1",
+    "cross-env": "7.0.3",
+    "glob": "7.1.7",
+    "npm-run-all": "4.1.5"


Same as above, keep only the ones needed to compile the theme specific styles.

brummbar · 2021-08-09T15:55:28Z

kits/default/bcl-builder.config.js

+  copy: [
+    {
+      from: ["node_modules/@openeuropa/bcl-theme-default/css/**"],
+      to: path.resolve(outputFolder, "assets/css"),
+      options: { up: true },
+    },
+    {
+      from: ["node_modules/@openeuropa/bcl-theme-default/js/**"],
+      to: path.resolve(outputFolder, "assets/js"),
+      options: { up: true },
+    },
+    {
+      from: ["node_modules/@openeuropa/bcl-theme-default/icons/bootstrap-icons.svg"],
+      to: path.resolve(outputFolder, "assets/icons"),
+      options: { up: true },
+    },
+  ],


This we can drop.

brummbar · 2021-08-09T16:18:40Z

composer.json

@@ -7,6 +7,7 @@
    "prefer-stable": true,
    "require": {
        "php": ">=7.3",
+        "drupal/components": "^2.4",


We actually don't need this module. Since we are placing the bcl files under the /template/bcl folder, you can access them by doing @oe_bootstrap_theme/bcl.

drupal/components module is needed as @oe-bcl namespace is used within the components, see for ex: https://github.com/openeuropa/bootstrap-component-library/blob/0eba27415245ae083fb9572417f1911960aa98e4/src/components/bcl-accordion/accordion.html.twig#L48

Yep, my bad! I didn't see that, and forgot about oe_theme 😄 I'll create a custom twig loader for this project to expose this namespace. Enabling the whole component module to have just its loader is too much.

Actually, the components module handles only namespaces, so it's the correct fit for this. 2x my bad 😄
I'll restore the module.

Removed the commit that was dropping the components module.

brummbar · 2021-08-09T16:19:01Z

includes/page_attachments.inc

-    . '<script src="https://cdn.jsdelivr.net/gh/nuxodin/ie11CustomProperties@4.1.0/ie11CustomProperties.min.js"></script>'
-    . '<script crossorigin="anonymous" src="https://polyfill.io/v3/polyfill.min.js"></script>'
-    . '<script src="https://code.jquery.com/jquery-3.5.1.slim.min.js"></script>'
-    . '<script>!function () { var e, t; ((e = document.createEvent("CustomEvent")).initEvent("Bootstrap", !0, !0), e.preventDefault(), e.defaultPrevented) || (t = Event.prototype.preventDefault, Event.prototype.preventDefault = function () { this.cancelable && (t.call(this), Object.defineProperty(this, "defaultPrevented", { get: function () { return !0 }, configurable: !0 })) }) }();</script>';
+    . '<script src="https://cdn.jsdelivr.net/combine/npm/bootstrap@5.0.0-beta2/dist/js/bootstrap.bundle.min.js,npm/ie11-custom-properties@4,npm/element-qsa-scope@1"></script>'
+    . '<script crossorigin="anonymous" src="https://polyfill.io/v3/polyfill.min.js?features=default%2CNumber.parseInt%2CNumber.parseFloat%2CArray.prototype.find%2CArray.prototype.includes"></script>';


We will rework this in a next iteration.

brummbar · 2021-08-09T16:53:10Z

kits/default/package.json

+    "@openeuropa/bcl-builder": "0.4.0",
+    "chokidar-cli": "1.2.0",
+    "copyfiles": "2.4.1",
+    "cross-env": "7.0.3",
+    "glob": "7.1.7",
+    "npm-run-all": "4.1.5"


We need also to look at these dependencies, move to chokidar 3.x (I tried locally and it seemed to work), and not lock dependencies to a specific patch if not needed.

Comments adressed

brummbar self-requested a review July 12, 2021 13:11

drishu requested changes Jul 28, 2021

View reviewed changes

papegaill requested a review from drishu July 28, 2021 15:09

drishu reviewed Jul 30, 2021

View reviewed changes

docker-compose.yml Show resolved Hide resolved

drishu reviewed Jul 30, 2021

View reviewed changes

kits/default/bcl-builder.config.js Outdated Show resolved Hide resolved

drishu approved these changes Jul 30, 2021

View reviewed changes

drishu previously requested changes Aug 3, 2021

View reviewed changes

kits/default/default.info.hidden.yml Outdated Show resolved Hide resolved

kits/default/default.info.hidden.yml Outdated Show resolved Hide resolved

kits/default/default.info.hidden.yml Show resolved Hide resolved

papegaill requested a review from drishu August 6, 2021 11:59

papegaill added 20 commits August 9, 2021 18:20

FRONT-2362: Add bcl-theme-default package to the base theme and updat…

ce2b16a

…e bootstrap-ie11 to 5.0.1.

FRONT-2362: Replace laravelmix with bcl-builder for base and sub-theme.

1de2806

FRONT-2362: Add components module.

3696a1d

FRONT-2362: Update BCL library to v.0.2.0.

4510cc3

FRONT-2362: Refactor Sub-theme Libraries.

1a98ccd

FRONT-2362: Fix bcl-builder copyfiles nested folder issues.

2720662

FRONT-2362: Update BCL to latest 0.3.0 version.

782cb18

FRONT-2362: Update BCL to 0.4.0 version.

251d683

FRONT-2362: Use npm prepublishOnly instead of deprecated prepublish c…

1707338

…ommand.

FRONT-2362: Fix bcl-builder icon path.

8b271ac

FRONT-2362: Add production npm command.

b799b4c

FRONT-2362: Use postinstall instead of prepublishOnly npm command.

e4da4d8

FRONT-2362: Add watch task for the sub-theme and update readme file.

19c382e

FRONT-2362: Remove postinstall npm script and update main readme file.

3cb76f3

FRONT-2362: Add node service in the docker-compose file.

42606b3

FRONT-2362: Reorder services in docker-compose file.

c6a5258

FRONT-2362: Remove jquery-3.6.0.slim.js that is already loaded from d…

d120d28

…rupal core.

FRONT-2362: Remove failing stylelint in watch task.

3539e4e

FRONT-2362: Enable components module in runner.yml.dist file.

ea41870

FRONT-2362: Update readme files.

b9a7bda

papegaill and others added 4 commits August 9, 2021 18:26

FRONT-2362: Replace oe_bootstrap_theme with placeholder, add it also …

16c594b

…to .libraries.yml.

FRONT-2362: Fix theme style css path in the base theme libraries.yml …

48e099a

…file.

Update kits/default/default.info.hidden.yml

effd24a

Co-authored-by: Szilagyi Andras <aszilagyi@live.com>

FRONT-2362: Remove deprecated stylesheets-remove.

a0407ae

brummbar force-pushed the FRONT-2362 branch from 699c4c3 to a0407ae Compare August 9, 2021 16:27

brummbar requested changes Aug 9, 2021

View reviewed changes

brummbar added 2 commits August 9, 2021 18:29

FRONT-2362: Drop selenium as it doesn't work on Drone for now.

822bdce

FRONT-2362: Make the subtheme reuse libraries from base theme.

b99e88e

brummbar force-pushed the FRONT-2362 branch from b021a12 to 3634040 Compare August 9, 2021 16:51

brummbar requested changes Aug 9, 2021

View reviewed changes

brummbar added 3 commits August 11, 2021 08:54

FRONT-2362: Bcl is not compiled anymore in subtheme.

ce33571

FRONT-2362: Syncronize main theme and subtheme package.json.

f386bdb

FRONT-2362: Update chokidar dependency.

dba3ca9

brummbar force-pushed the FRONT-2362 branch from 1a46a58 to dba3ca9 Compare August 11, 2021 07:19

FRONT-2362: Set correct dependencies for the helper module.

f5a6fb1

brummbar approved these changes Aug 11, 2021

View reviewed changes

claudiu-cristea approved these changes Aug 11, 2021

View reviewed changes

brummbar merged commit a19ae7e into 1.x Aug 11, 2021

brummbar deleted the FRONT-2362 branch August 11, 2021 12:49

papegaill mentioned this pull request Sep 2, 2021

Theme branding #39

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FRONT-2362: Refactor theme and sub-theme to use bcl-builder. #10

FRONT-2362: Refactor theme and sub-theme to use bcl-builder. #10

papegaill commented Jul 12, 2021 •

edited

Loading

drishu left a comment

papegaill commented Jul 28, 2021

drishu left a comment

papegaill commented Aug 6, 2021

brummbar Aug 9, 2021

brummbar Aug 9, 2021

papegaill Aug 10, 2021

brummbar Aug 9, 2021

papegaill Aug 10, 2021

brummbar Aug 9, 2021

brummbar Aug 9, 2021

brummbar Aug 9, 2021

papegaill Aug 10, 2021

brummbar Aug 10, 2021

brummbar Aug 11, 2021

brummbar Aug 11, 2021

brummbar Aug 9, 2021

brummbar Aug 9, 2021

		base theme: oe_bootstrap_theme
		base theme: OE_BOOTSTRAP_THEME_SUBTHEME_MACHINE_NAME

FRONT-2362: Refactor theme and sub-theme to use bcl-builder. #10

FRONT-2362: Refactor theme and sub-theme to use bcl-builder. #10

Conversation

papegaill commented Jul 12, 2021 • edited Loading

drishu left a comment

Choose a reason for hiding this comment

papegaill commented Jul 28, 2021

drishu left a comment

Choose a reason for hiding this comment

papegaill commented Aug 6, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

papegaill commented Jul 12, 2021 •

edited

Loading