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

User provided styles for toolbar and editor containers #53

Merged
merged 1 commit into from
Jun 14, 2016

Conversation

ralphschindler
Copy link
Contributor

@ralphschindler ralphschindler commented May 21, 2016

Allow the appending of user provided classNames to the toolbar container and the editor container divs.

@ralphschindler
Copy link
Contributor Author

Hey @sstur, while this gets me part of the way there, there is still an issue. Because the library is using css-modules without the extract-text plugin, styles are being appened to head via javascript. In an of itself that is not a problem, but when attempting to use local styles compiled to a file which is linked in head, the library styles are appended in head, meaning their styles, if selected, "win" in terms of css priority.

I would have expected those styles to be compiled into my own css file in my project, but I cannot figure out how to do that (if that is a webpack/css-module loader feature). Do you know a way I can ensure library provided css-modules are combined with my own css-modules in my project, and if they are, how I can ensure order? I have not been able to find many "libraries" that use css-modules (it appears most are still using their own hand coded css, with their own prefixes.

I look forward to your thoughts. And if you have any best-practice resources on using css-modules in libraries, I'd love to read through them :)

@sstur
Copy link
Owner

sstur commented May 22, 2016

This looks good to me.

"Because the library is using css-modules without the extract-text plugin, styles are being appended to head"

I would be open to using the extract-text plugin but I'm not too sure the best way to proceed. I'd like to minimize developer lock-in as much as possible. Meaning I don't want to decide for people how styles should be inserted, but I'd also like to have a sensible default so people who don't know or care about CSS modules can get up and going without making any decisions there.

I'd love to get an opinion from the creators of CSS Modules about the best way to proceed. I've run into other issues too, like allowing people to use parts of this project in their own build system that doesn't know how to require('./foo.css'). I feel there should be a way to transform the css to normal js files that can be required. But that's another issue really.

Overall, I've been extremely happy with CSS Modules. Maybe it needs to mature a little or I just need to learn how to work around these challenges.

Sorry I don't have any better solutions for you about this. Maybe I can try to tweet at @markdalgleish and chat with him about some of these challenges.

@ralphschindler ralphschindler changed the title [WIP] User provided styles User provided styles for toolbar and editor containers May 25, 2016
@ralphschindler
Copy link
Contributor Author

I think this PR is a good-enough PR that gives consumers some avenue to attempt to inject their own styles. For me, I have to use compound / more specific styles to override the base styles (like .someHigherDiv .toolbarDiv), since the vendor styles are appended last in head... but that is ok.

If you come across any ways to better incorporate css-modules in a way that consumers can choose how to write them to their apps, would love to see that in action. Thanks again.

@sstur
Copy link
Owner

sstur commented May 25, 2016

OK, Thanks! I'll take one last look at this and then get it landed.

One comment about your CSS specificity issue, there's a trick that you can double a class name to increase specificity. like .foo.foo { ... } (with no space between). You can triple it if necessary and the specificity goes up linearly.

@ralphschindler
Copy link
Contributor Author

Thanks!

Yes, "increasing specificity"... that is what I was attempting to describe with "compound / more specific styles" (I am not a CSS master), haha. Cheers.

@ralphschindler
Copy link
Contributor Author

Hi @sstur - is there anything more you need from me to merge and release? Thanks in advance and let me know if there is anything I can do. -ralph

@oeddyo
Copy link

oeddyo commented Jun 13, 2016

If possible, can we merge this?

@sstur
Copy link
Owner

sstur commented Jun 14, 2016

Sorry for the delay on this. This is good to be merged. Thanks for the contribution!

@sstur sstur merged commit c1962ae into sstur:master Jun 14, 2016
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

3 participants