From 22db5fe5c0c0fb3318065b74e346da1c28d0aafa Mon Sep 17 00:00:00 2001 From: Cockpituous Date: Thu, 7 May 2020 22:18:37 +0000 Subject: [PATCH] Stop importing cockpit's base1/patternfly.css This is deprecated API and will be dropped at some point, in favor of projects shipping their own CSS. Install and import the styles from PF4 now. Update the travis config to run `make`, as `npm run build` by itself doesn't generate the file. Closes #315 --- .travis.yml | 3 +- Makefile | 9 +++++- package.json | 7 +++-- src/app.jsx | 20 ++++++++++---- src/index.html | 3 +- src/index.js | 10 +++++++ src/lib/_fonts.scss | 36 ++++++++++++++++++++++++ src/lib/patternfly-4-cockpit.scss | 14 ++++++++++ src/lib/patternfly-4-overrides.scss | 43 +++++++++++++++++++++++++++++ src/lib/patternfly.sed | 3 ++ test/check-application | 8 +++--- webpack.config.js | 19 +++++++++---- 12 files changed, 153 insertions(+), 22 deletions(-) create mode 100644 src/lib/_fonts.scss create mode 100644 src/lib/patternfly-4-cockpit.scss create mode 100644 src/lib/patternfly-4-overrides.scss create mode 100644 src/lib/patternfly.sed diff --git a/.travis.yml b/.travis.yml index e5d53fc..2d4a7b1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,5 +4,4 @@ language: node_js node_js: - "8" script: - - npm install - - npm run build + - make diff --git a/Makefile b/Makefile index ae129f5..8c04110 100644 --- a/Makefile +++ b/Makefile @@ -12,6 +12,7 @@ VM_IMAGE=$(CURDIR)/test/images/$(TEST_OS) NODE_MODULES_TEST=package-lock.json # one example file in dist/ from webpack to check if that already ran WEBPACK_TEST=dist/index.html +SASS=$(CURDIR)/node_modules/.bin/sass all: $(WEBPACK_TEST) @@ -65,7 +66,7 @@ dist/po.%.js: po/%.po $(NODE_MODULES_TEST) %.spec: %.spec.in sed -e 's/%{VERSION}/$(VERSION)/g' $< > $@ -$(WEBPACK_TEST): $(NODE_MODULES_TEST) $(shell find src/ -type f) package.json webpack.config.js $(patsubst %,dist/po.%.js,$(LINGUAS)) +$(WEBPACK_TEST): $(NODE_MODULES_TEST) $(shell find src/ -type f) package.json webpack.config.js dist/patternfly-4-cockpit.css $(patsubst %,dist/po.%.js,$(LINGUAS)) NODE_ENV=$(NODE_ENV) npm run build watch: @@ -75,9 +76,11 @@ clean: rm -rf dist/ [ ! -e cockpit-$(PACKAGE_NAME).spec.in ] || rm -f cockpit-$(PACKAGE_NAME).spec +# patternfly-4-cockpit.css needs to be shipped in the release tarball to avoid rebuilding the webpack, but we don't need to install it install: $(WEBPACK_TEST) mkdir -p $(DESTDIR)/usr/share/cockpit/$(PACKAGE_NAME) cp -r dist/* $(DESTDIR)/usr/share/cockpit/$(PACKAGE_NAME) + rm $(DESTDIR)/usr/share/cockpit/$(PACKAGE_NAME)/patternfly-4-cockpit.css mkdir -p $(DESTDIR)/usr/share/metainfo/ cp org.cockpit-project.$(PACKAGE_NAME).metainfo.xml $(DESTDIR)/usr/share/metainfo/ @@ -156,6 +159,10 @@ test/common: git checkout --force FETCH_HEAD -- test/common git reset test/common +dist/patternfly-4-cockpit.css: src/lib/patternfly-4-cockpit.scss $(NODE_MODULES_TEST) + $(SASS) $< $@ --source-map --source-map-urls absolute --style compressed --load-path node_modules && \ + sed -i -f src/lib/patternfly.sed $@ + $(NODE_MODULES_TEST): package.json # if it exists already, npm install won't update it; force that so that we always get up-to-date packages rm -f package-lock.json diff --git a/package.json b/package.json index d9ab2b1..680c864 100644 --- a/package.json +++ b/package.json @@ -33,10 +33,12 @@ "eslint-plugin-react": "^7.14.3", "eslint-plugin-react-hooks": "^2.1.2", "eslint-plugin-standard": "^4.0.1", - "mini-css-extract-plugin": "^0.9.0", "htmlparser": "^1.7.7", "jed": "^1.1.1", + "mini-css-extract-plugin": "^0.9.0", + "node-sass": "4.14.1", "po2json": "^1.0.0-alpha", + "sass": "^1.26.5", "sass-loader": "^7.0.3", "sizzle": "^2.3.3", "stdio": "^0.2.7", @@ -44,8 +46,9 @@ "webpack-cli": "^3.1.0" }, "dependencies": { + "@patternfly/patternfly": "^2.71.6", + "@patternfly/react-core": "^3.158.1", "core-js": "3.6.5", - "node-sass": "4.14.1", "react": "16.13.1", "react-dom": "16.13.1" } diff --git a/src/app.jsx b/src/app.jsx index 1f671a9..9f17f50 100644 --- a/src/app.jsx +++ b/src/app.jsx @@ -19,6 +19,7 @@ import cockpit from 'cockpit'; import React from 'react'; +import { Alert, Card, CardHead, CardHeader, CardHeadMain, Title } from '@patternfly/react-core'; import './app.scss'; const _ = cockpit.gettext; @@ -35,12 +36,19 @@ export class Application extends React.Component { render() { return ( -
-

Starter Kit

-

- { cockpit.format(_("Running on $0"), this.state.hostname) } -

-
+ + + + Starter Kit + + + + + + ); } } diff --git a/src/index.html b/src/index.html index 5182d9b..62a2bf0 100644 --- a/src/index.html +++ b/src/index.html @@ -22,7 +22,6 @@ along with this package; If not, see . - @@ -30,7 +29,7 @@ along with this package; If not, see . - +
diff --git a/src/index.js b/src/index.js index 0a0e535..4981382 100644 --- a/src/index.js +++ b/src/index.js @@ -17,11 +17,21 @@ * along with Cockpit; If not, see . */ +import "../dist/patternfly-4-cockpit.css"; + import "core-js/stable"; import React from 'react'; import ReactDOM from 'react-dom'; import { Application } from './app.jsx'; +/* + * PF4 overrides need to come after the JSX components imports because + * these are importing CSS stylesheets that we are overriding + * Having the overrides here will ensure that when mini-css-extract-plugin will extract the CSS + * out of the dist/index.js and since it will maintain the order of the imported CSS, + * the overrides will be correctly in the end of our stylesheet. + */ +import "./lib/patternfly-4-overrides.scss"; document.addEventListener("DOMContentLoaded", function () { ReactDOM.render(React.createElement(Application, {}), document.getElementById('app')); diff --git a/src/lib/_fonts.scss b/src/lib/_fonts.scss new file mode 100644 index 0000000..b3cb18a --- /dev/null +++ b/src/lib/_fonts.scss @@ -0,0 +1,36 @@ +/* + * Keep in sync with https://github.com/cockpit-project/cockpit/tree/master/src/base1/_fonts.scss + */ + +@mixin printRedHatFont( +$weightValue: 400, +$weightName: "Regular", +$familyName: "RedHatText", +$style: "normal", +$relative: true +) { + $filePath: "../../static/fonts" + "/" + $familyName + "-" + $weightName; + @font-face { + font-family: $familyName; + src: url('#{$filePath}.woff2') format('woff2'); + font-style: #{$style}; + font-weight: $weightValue; + text-rendering: optimizeLegibility; + } +} + +@include printRedHatFont(700, "Bold", $familyName: "RedHatDisplay"); +@include printRedHatFont(700, "BoldItalic", $style: "italic", $familyName: "RedHatDisplay"); +@include printRedHatFont(300, "Black", $familyName: "RedHatDisplay"); +@include printRedHatFont(300, "BlackItalic", $style: "italic", $familyName: "RedHatDisplay"); +@include printRedHatFont(300, "Italic", $style: "italic", $familyName: "RedHatDisplay"); +@include printRedHatFont(400, "Medium", $familyName: "RedHatDisplay"); +@include printRedHatFont(400, "MediumItalic", $style: "italic", $familyName: "RedHatDisplay"); +@include printRedHatFont(300, "Regular", $familyName: "RedHatDisplay"); + +@include printRedHatFont(300, "Bold"); +@include printRedHatFont(300, "BoldItalic", $style: "italic"); +@include printRedHatFont(300, "Italic"); +@include printRedHatFont(700, "Medium"); +@include printRedHatFont(700, "MediumItalic", $style: "italic"); +@include printRedHatFont(400, "Regular"); diff --git a/src/lib/patternfly-4-cockpit.scss b/src/lib/patternfly-4-cockpit.scss new file mode 100644 index 0000000..d4af1b8 --- /dev/null +++ b/src/lib/patternfly-4-cockpit.scss @@ -0,0 +1,14 @@ +/* + * Keep in sync with https://github.com/cockpit-project/cockpit/tree/master/src/base1/patternfly-4-cockpit.scss + */ + +/* Set fake font and icon path variables - we are going to indentify these through + * patternfly.sed and remove the relevant font-face declarations + */ +$pf-global--font-path: 'patternfly-fonts-fake-path'; +$pf-global--fonticon-path: 'patternfly-icons-fake-path'; +$pf-global--disable-fontawesome: true !default; // Disable Font Awesome 5 Free +@import '@patternfly/patternfly/patternfly-base.scss'; + +/* Import our own fonts since the PF4 font-face rules are filtered out with patternfly.sed */ +@import "./fonts"; diff --git a/src/lib/patternfly-4-overrides.scss b/src/lib/patternfly-4-overrides.scss new file mode 100644 index 0000000..a02cb95 --- /dev/null +++ b/src/lib/patternfly-4-overrides.scss @@ -0,0 +1,43 @@ +/* + * Keep in sync with https://github.com/cockpit-project/cockpit/tree/master/pkg/lib/patternfly-4-overrides.scss + */ + +/*** PF4 overrides ***/ + +/* WORKAROUND: Override word-break bug */ +/* See: https://github.com/patternfly/patternfly-next/issues/2325 */ +.pf-c-table td { + word-break: normal; + overflow-wrap: break-word; +} + +/* WORKAROUND: Dropdown (PF4): Caret is not properly aligned bug */ +/* See: https://github.com/patternfly/patternfly/issues/2715 */ +/* Align the icons inside of all dropdown toggles. */ +/* Part 1 of 2 */ +.pf-c-dropdown__toggle-button { + display: flex; + align-items: center; +} + +/* Make split button dropdowns the same height as their sibling. */ +/* Part 2 of 2 */ +.pf-m-split-button { + align-items: stretch; +} + +/* WORKAROUND: Navigation problems with Tertiary Nav widget on mobile */ +/* See: https://github.com/patternfly/patternfly-design/issues/840 */ +/* Helper mod to wrap pf-c-nav__tertiary */ +.ct-m-nav__tertiary-wrap { + flex-wrap: wrap; + + .pf-c-nav__scroll-button { + display: none; + } +} + +/* Helper mod to center pf-c-nav__tertiary when it wraps */ +.ct-m-nav__tertiary-center { + justify-content: center; +} diff --git a/src/lib/patternfly.sed b/src/lib/patternfly.sed new file mode 100644 index 0000000..89d878c --- /dev/null +++ b/src/lib/patternfly.sed @@ -0,0 +1,3 @@ +# Keep the PF4 relevant rules in sync with https://github.com/cockpit-project/cockpit/blob/master/tools/patternfly.sed +s/src:url[(]"patternfly-icons-fake-path\/pficon[^}]*/src:url('fonts\/patternfly.woff')format('woff');/ +s/@font-face[^}]*patternfly-fonts-fake-path[^}]*}//g diff --git a/test/check-application b/test/check-application index 2ad5d68..c4068b2 100755 --- a/test/check-application +++ b/test/check-application @@ -20,16 +20,16 @@ class TestApplication(testlib.MachineCase): self.login_and_go("/starter-kit") # verify expected heading - b.wait_text(".container-fluid h2", "Starter Kit") + b.wait_text("h2.pf-c-title", "Starter Kit") # verify expected host name hostname = m.execute("hostname").strip() - b.wait_text(".container-fluid p", "Running on " + hostname) + b.wait_in_text("h4.pf-c-alert__title", "Running on " + hostname) # change current hostname m.execute("echo new-%s > /etc/hostname" % hostname) # verify new hostname name - b.wait_text(".container-fluid p", "Running on new-" + hostname) + b.wait_in_text("h4.pf-c-alert__title", "Running on new-" + hostname) # change language to German b.switch_to_top() @@ -48,7 +48,7 @@ class TestApplication(testlib.MachineCase): b.go("/starter-kit") b.enter_page("/starter-kit") # page label (from js) should be translated - b.wait_in_text(".container-fluid p", "Läuft auf") + b.wait_in_text("h4.pf-c-alert__title", "Läuft auf") if __name__ == '__main__': testlib.test_main() diff --git a/webpack.config.js b/webpack.config.js index 01973c4..3ecc95a 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -115,6 +115,9 @@ var babel_loader = { module.exports = { mode: production ? 'production' : 'development', + resolve: { + modules: [ nodedir ], + }, entry: info.entries, externals: externals, output: output, @@ -133,19 +136,25 @@ module.exports = { test: /\.(js|jsx)$/ }, { - exclude: /node_modules/, - test: /\.scss$/, + test: /\.s?css$/, use: [ extract.loader, { loader: 'css-loader', - options: { url: false } + options: { + sourceMap: true, + url: false + } }, { loader: 'sass-loader', - } + options: { + sourceMap: true, + outputStyle: 'compressed', + }, + }, ] - } + }, ] }, plugins: plugins