Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Carbon - add carbon scss #7068

Merged
merged 4 commits into from
May 22, 2020
Merged

Carbon - add carbon scss #7068

merged 4 commits into from
May 22, 2020

Conversation

himdel
Copy link
Contributor

@himdel himdel commented May 21, 2020

(Split off from #6963 as it was getting too large.)

This enables carbon css without affecting anything.
This PR should cause no visual differences at all :).

ui-shell css is needed for the new menu (#6963),

not switching to IBM fonts globally because:

also #5679 - switching reset stylesheets can wait, and body styling and typography can change after that.

Overriding html font-size to 100% to get reasonable styling from carbon use of rem units, patternfly sets body to 12px.

Issue: #6817

cc @skateman

and disabling IBM fonts for now, until we get patternfly over webpack

after that, https://github.com/bholloway/resolve-url-loader/blob/master/packages/resolve-url-loader/README.md
we may need to use a custom `join` function to change font urls from fonts.gstatic.com
setting the html font size changes the rem units to the actual carbon defaults,
so we get some menu sizing for free

patternfly overrides body font-size, so this doesn't break patternfly styling
@miq-bot
Copy link
Member

miq-bot commented May 21, 2020

Checked commits https://github.com/himdel/manageiq-ui-classic/compare/2ce268e42ba9968bf9b02148ac6e163f89e579df~...0affc237f4a80924a43626dc798b3ae162eb5b4d with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
0 files checked, 0 offenses detected
Everything looks fine. 🍰

@skateman
Copy link
Member

Is this changing anything visually?

@himdel
Copy link
Contributor Author

himdel commented May 22, 2020

No, it shouldn't change anything visually.

@skateman
Copy link
Member

I assume you're aware of these new warnings:

WARNING in ./app/stylesheet/application-webpack.scss (./node_modules/css-loader/dist/cjs.js!./node_modules/postcss-loader/src??ref--7-2!./node_modules/resolve-url-loader!./node_modules/sass-loader/dist/cjs.js??ref--7-4!./app/stylesheet/application-webpack.scss)
Module Warning (from ./node_modules/postcss-loader/src/index.js):
Warning

(9155:3) IE supports only grid-row with / and span. You should add grid: false option to Autoprefixer and use some JS grid polyfill for full spec support
 @ ./app/stylesheet/application-webpack.scss 2:26-264
 @ ./app/javascript/packs/application-common.js

WARNING in ./app/stylesheet/application-webpack.scss (./node_modules/css-loader/dist/cjs.js!./node_modules/postcss-loader/src??ref--7-2!./node_modules/resolve-url-loader!./node_modules/sass-loader/dist/cjs.js??ref--7-4!./app/stylesheet/application-webpack.scss)
Module Warning (from ./node_modules/postcss-loader/src/index.js):
Warning

(9178:3) IE supports only grid-row with / and span. You should add grid: false option to Autoprefixer and use some JS grid polyfill for full spec support
 @ ./app/stylesheet/application-webpack.scss 2:26-264
 @ ./app/javascript/packs/application-common.js

WARNING in ./app/stylesheet/application-webpack.scss (./node_modules/css-loader/dist/cjs.js!./node_modules/postcss-loader/src??ref--7-2!./node_modules/resolve-url-loader!./node_modules/sass-loader/dist/cjs.js??ref--7-4!./app/stylesheet/application-webpack.scss)
Module Warning (from ./node_modules/postcss-loader/src/index.js):
Warning

(9199:3) IE supports only grid-row with / and span. You should add grid: false option to Autoprefixer and use some JS grid polyfill for full spec support
 @ ./app/stylesheet/application-webpack.scss 2:26-264
 @ ./app/javascript/packs/application-common.js

WARNING in ./app/stylesheet/application-webpack.scss (./node_modules/css-loader/dist/cjs.js!./node_modules/postcss-loader/src??ref--7-2!./node_modules/resolve-url-loader!./node_modules/sass-loader/dist/cjs.js??ref--7-4!./app/stylesheet/application-webpack.scss)
Module Warning (from ./node_modules/postcss-loader/src/index.js):
Warning

(9220:3) IE supports only grid-row with / and span. You should add grid: false option to Autoprefixer and use some JS grid polyfill for full spec support
 @ ./app/stylesheet/application-webpack.scss 2:26-264
 @ ./app/javascript/packs/application-common.js

@skateman skateman self-assigned this May 22, 2020
Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Seal of Approval

@skateman skateman merged commit 9cfcf1c into ManageIQ:master May 22, 2020
@himdel himdel deleted the carbon-base branch May 22, 2020 14:19
@himdel
Copy link
Contributor Author

himdel commented May 22, 2020

I assume you're aware of these new warnings

Thanks, fixed in #7072 :)

@himdel himdel mentioned this pull request May 26, 2020
simaishi pushed a commit that referenced this pull request May 28, 2020
Carbon - add carbon scss

(cherry picked from commit 9cfcf1c)
@simaishi
Copy link
Contributor

Jansa backport details:

$ git log -1
commit ce437531cfa8003350cb678ed7a05479e3781d11
Author: Halász Dávid <skateman@users.noreply.github.com>
Date:   Fri May 22 16:09:09 2020 +0200

    Merge pull request #7068 from himdel/carbon-base

    Carbon - add carbon scss

    (cherry picked from commit 9cfcf1ca5f5d21a4403e12275fc84c3b443f74ee)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants