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

Fix leaking antd global CSS rules #19

Merged

Conversation

Jokinen
Copy link
Contributor

@Jokinen Jokinen commented Aug 15, 2018

Ant design imports some global CSS styles in its library index. This
means that all library users importing directly from ant design's root,
e.g. "import {} from 'antd'", will include these global reset styles.

This commit does its best to avoid polluting global rules by

  1. avoiding imports from ant design's package root when possible,
  2. implementing special build processes for development and library
    builds which preface ant design's global styles.

This is a somewhat hacky fix, but maybe it can act as a starting point. I needed some way to stop the global styles from leaking for one of our projects.

What I've done is

  1. Removed imports referencing antd directly
  2. Added a plugin for development builds which prefaces all of the antd "core" styles with an id
  3. Added a build script which does everything as it did before, but now also compiles the antd core styles and adds an import statement for those in lib/index.js
  4. Added RBS-Scheduler-root id for the <Scheduler /> component

I haven't had time to proof this approach much so I don't know how valid it is. I didn't implement a webpack build for production as it would bundle all the files which could be problematic(?). Instead I've only added a step to the current process, but it's still a rather big change.

#20

Ant design imports some global CSS styles in its library index. This
means that all library users importing directly from ant design's root,
e.g. "import {} from 'antd'", will include these global reset styles.

This commit does its best to avoid polluting global rules by
1. avoiding imports from ant design's package root when possible,
2. implementing special build processes for development and library
   builds which preface ant design's global styles.
Copy link

@adeacetis adeacetis left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for taking care of this. Hopefully, this will get merged rather sooner than later,

@tgBryanBailes
Copy link
Contributor

@StephenChou1017 Please consider merging this quickly. Currently, react-big-scheduler is unusable because it intrusively changes your entire site's styling.

@StephenChou1017
Copy link
Owner

OK, I'll try to merge this, I haven't merged this for @Jokinen said he didn't proof this approach:-)

@StephenChou1017 StephenChou1017 merged commit dcc447c into StephenChou1017:master Oct 30, 2018
@StephenChou1017
Copy link
Owner

@Jokinen I've merged your code, thanks for your pr:-)

@Jokinen
Copy link
Contributor Author

Jokinen commented Oct 30, 2018

Np, glad if I could help 👏

@Jokinen Jokinen deleted the fix-leaking-antd-globals branch October 30, 2018 10:46
@StephenChou1017
Copy link
Owner

StephenChou1017 commented Nov 7, 2018

@Jokinen
Hi, I've replaced all code like import {A} from 'antd' with import A from 'antd/lib/a'; import 'antd/lib/a/style/index.css', and pushed the new code. But there still are styling errors in the local examples after I run the npm run example command.
error

I wanna to find out how to use the Scheduler component in the following two cases:

  1. All web site pages are antd style compatible and use antd global CSS rules;
    2.The web site pages have their own global CSS rules, only a few pages use the Scheduler component;

If I add import 'antd/lib/style/index.less' in src\index.js(or in example\Basic.js), it displays correctly. Should I add import 'antd/lib/style/index.less' in src\index.js?

@Jokinen
Copy link
Contributor Author

Jokinen commented Nov 7, 2018

Hi!

I don’t have the means to test it out rigth now, but I think that if you include that statement, the global styles will get included for everyone using your library 🙂

In essence, RBS should always be styled correctly whether antd is used or not. If not, then these faults should be considered as bugs. Note that the application surrounding RBS would not be styled correctly.

Any major styling errors which are caused by missing antd global rules shoul be addressed in the CSS rules defined for react-big-scheduler!

In case 1
you include the global styles in your own project if you need them. In the case of the example app, you would import the global styles there example/), for instance by importing directly from antd or importing a css(/less if preprocessor is in your build) file which contains the global styles.

BUT, you shouldn’t need to do that to make RBS work!

In case 2
it should work out-of-the-box.

I’m not actually sure which things are not correct in the screencap! 😄 Should the scroll bars be hidden?

@StephenChou1017
Copy link
Owner

@Jokinen
Hi, I think I've got you a bit, I'll publish one beta version to test it.

In the screencap, the table is abnormal.

@StephenChou1017
Copy link
Owner

StephenChou1017 commented Nov 23, 2018

@Jokinen @adeacetis @tgBryanBailes
Hi, I've just published 0.2.4, and it's mainly about this pr.

@beysong
Copy link

beysong commented Dec 27, 2018

I am using Antd Pro 2.0, and leaking the global css umi.css also.

Without from option PostCSS could generate wrong source map and will not find Browserslist config. Set it to CSS file path or to undefined to preven
t this warning.

@StephenChou1017
Copy link
Owner

I am using Antd Pro 2.0, and it seems normal. I use antd@3.11.6 and react-big-scheduler@0.2.5 in pro.

@beysong
Copy link

beysong commented Dec 28, 2018

I am using Antd Pro 2.0, and it seems normal. I use antd@3.11.6 and react-big-scheduler@0.2.5 in pro.

I found "npm run build" is ok, but in develop mode "npm start" will missing umi.css.

thanks you @StephenChou1017

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

Successfully merging this pull request may close these issues.

None yet

5 participants