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

Add support for PF4 styling through webpack #5537

Merged
merged 1 commit into from May 9, 2019

Conversation

skateman
Copy link
Member

@skateman skateman commented May 6, 2019

To avoid collisions with the old world, the new version is loaded through webpack instead of the asset pipeline. Reopen of #4841 as I accidentally deleted the branch.

@miq-bot add_label hammer/no, changelog/yes, graphics
@miq-bot add_reviewer @epwinchell
@miq-bot add_reviewer @himdel

@@ -22,6 +22,8 @@ import { history } from '../miq-component/react-history.ts';
import createReduxRoutingActions from '../miq-redux/redux-router-actions';
import { formButtonsActionTypes, createFormButtonsActions } from '../forms/form-buttons-reducer';

import '../../assets/stylesheets/application-webpack.scss';
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we start separating the CSS too?

(Importing app/assets/stylesheets/ from webpack is surprising, let's not do that for any JS, but with css...
¯\_(ツ)_/¯ up to you :))

Copy link
Member Author

Choose a reason for hiding this comment

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

Ideally we would not need any custom CSS :trollface: but I get your point. Maybe /app/stylesheets?

Copy link
Contributor

@himdel himdel May 6, 2019

Choose a reason for hiding this comment

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

Works for me :) Thanks :)

(if you want to be 100% consistent, app/stylesheet, because asset pipeline has plurals but webpack has app/javascript. But both are fine :).)

(Or app/css because who remembers "stylesheet" :D But then somebody would add app/scss too :))

Copy link
Member Author

Choose a reason for hiding this comment

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

/app/stylesheet it is then

@epwinchell
Copy link
Contributor

epwinchell commented May 6, 2019

I'll paste in issues as I find them

Bef
Screen Shot 2019-05-06 at 1 51 05 PM

After
Screen Shot 2019-05-06 at 12 53 02 PM

Bef
Screen Shot 2019-05-06 at 1 48 55 PM

After
Screen Shot 2019-05-06 at 12 53 53 PM

@miq-bot
Copy link
Member

miq-bot commented May 9, 2019

Checked commit skateman@a1e4692 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🏆

Copy link
Contributor

@epwinchell epwinchell left a comment

Choose a reason for hiding this comment

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

Tested. Looks good now.

@himdel himdel self-assigned this May 9, 2019
@himdel himdel added this to the Sprint 111 Ending May 13, 2019 milestone May 9, 2019
@himdel
Copy link
Contributor

himdel commented May 9, 2019

Great, we still have no public announcement about PF4 being available in ivanchuk (and IE11 not),
but in this case, it's probably better to merge sooner than later 👍

@himdel himdel merged commit 1d421a8 into ManageIQ:master May 9, 2019
@skateman skateman deleted the pf-4-support branch May 9, 2019 17:09

@import '~@fortawesome/fontawesome-free/css/all.css';
@import '~@fortawesome/fontawesome-free/scss/v4-shims.scss';
@import '~@patternfly/patternfly-next/patternfly.scss';
Copy link
Contributor

Choose a reason for hiding this comment

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

should be @patternfly/patternfly/patternfly.scss - #5635

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it wasn't when I read the docs about it ...

Copy link
Contributor

Choose a reason for hiding this comment

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

Not blaming you, I still can't even find any docs about it :)

But looks like the last release of patternfly-next was in January, since then, it's the other one.

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.

None yet

4 participants