leejordan/reflex

Too aggressive text-align-last

kmkr opened this issue · 8 comments

kmkr commented

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

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

kmkr commented

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.

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 commented

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.

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

xla commented

@leejordan 👍 On smoothing the assumptions around text aligning.

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 commented

@kmkr @leejordan Thanks a lot! 🍡

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.