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

Use inline styles for general purpose layout #3

Merged
merged 1 commit into from Apr 1, 2019

Conversation

Projects
None yet
2 participants
@einarlove
Copy link
Contributor

commented Apr 1, 2019

I thought the CSS from toasted-notes/src/styles.css was optional, but I quickly found out that was not the case unless you add global Toast_* specific styles.

This pull request tried to remedy this by adding inline styles for the positioning of the notifications and leaving the decision on styling the notifications up to the developer.

I did not find a Prettier or ESLint config so not sure if the formatting is consistent with yours, and not proficient in TypeScript so not sure if I did it correctly.

@bmcmahen

This comment has been minimized.

Copy link
Owner

commented Apr 1, 2019

Thanks @einarlove! I think this is probably a good idea and it lowers the barrier to entry for using this library. On first glance, everything looks good (I'm no Typescript expert either), but I'll give it a more serious look over the coming days and get this merged.

@bmcmahen bmcmahen merged commit 890855b into bmcmahen:master Apr 1, 2019

@bmcmahen

This comment has been minimized.

Copy link
Owner

commented Apr 1, 2019

You rock! Thanks.

@einarlove einarlove deleted the einarlove:use-inline-styles branch Apr 2, 2019

@einarlove

This comment has been minimized.

Copy link
Contributor Author

commented Apr 2, 2019

Something to note in the readme, now that flex styles are applied inline instead via a .css file, most people have to way of auto-prefixing the styles. Meaning this library does not support Internet Explorer 10 and iOS < 2015 by default.

A remedy is to style the default layout themselves with the classnames, use a prefixing utility when we inline the styles, or use an alterntive layout method than flex like display inline-block with ltr/rlt.

@bmcmahen

This comment has been minimized.

Copy link
Owner

commented Apr 2, 2019

It's a good point. I'm personally not too concerned about supporting those browsers and, like you said, support is there via classnames and autoprefixing if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.