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

Too aggressive text-align-last #14

Closed
kmkr opened this issue Dec 14, 2015 · 8 comments
Closed

Too aggressive text-align-last #14

kmkr opened this issue Dec 14, 2015 · 8 comments

Comments

@kmkr
Copy link

kmkr commented Dec 14, 2015

Text-align-last value "left | start" assumes that children aligns everything to the left.

Example: http://plnkr.co/edit/n1za5W?p=preview

@kmkr
Copy link
Author

kmkr commented Dec 14, 2015

PR #15 is meant to fix this. I'm not sure why text-align-last was added in the first place, so please verify that the change do not break that before consider accepting the PR.

@leejordan
Copy link
Owner

Thanks for your pull request. The text-align reset is due to the way the inline-block fallback works with use of text-align. If you have a right aligned grid, you probably don't want the grid contents to inherit this, hence I reset everything to left/start.

text-align-last is the closest fallback possible for justify-content: space-between

Also I don't think text-align: auto is a valid css property. Can you explain further please?

https://developer.mozilla.org/en/docs/Web/CSS/text-align

I'll take a proper look in the near future.

@kmkr
Copy link
Author

kmkr commented Dec 14, 2015

The text-align reset is due to the way the inline-block fallback works with use of text-align. If you have a right aligned grid, you probably don't want the grid contents to inherit this, hence I reset everything to left/start.

Ok, I see.

Also I don't think text-align: auto is a valid css property. Can you explain further please?

That's correct, sorry. It was supposed to be unset for the text-align, and auto only for text-align-left. It's fixed in the PR. text-align-left set to auto should use the value from text-align that's unset. I believe this will do the same trick for you (setting to left) without forcing children to override the text-align-last.

@leejordan
Copy link
Owner

Thanks for explaining this sounds pretty good to me. I'll take a proper look and test it out in the future.

@xla
Copy link

xla commented Jan 4, 2016

@leejordan 👍 On smoothing the assumptions around text aligning.

@leejordan
Copy link
Owner

I've addressed this issue in a recent commit 758c9b4

showing fix based on your demo: http://plnkr.co/edit/TYwQkEi22YaGzkSsI6gF?p=preview

I've opted to use the "initial" property because it fixes the issue you demonstrated in your demo. It makes the text alignment reset much less aggressive and allows you to more easily have differently aligned elements within the grid. It also plays nicely with the fallback inline-block grid.

Thanks very much for taking the time to produce a demo of the problem. Let me know if you have any further issues. I'll be creating release 1.0.8 shortly which will contain this fix among others and I will update the npm version at the same time.

@xla
Copy link

xla commented Jan 5, 2016

@kmkr @leejordan Thanks a lot! 🍡

@pixeldesignstudio
Copy link

This issue is still present in the newest release.
My custom class .text-center doesn't work because of the aggressive selector.
!important is no option in my opinion.

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

No branches or pull requests

4 participants