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.

Sign up for GitHub

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

Jump to bottom

Remove display property from sizeFull #36

Merged
merged 2 commits into from
Jul 8, 2016
Merged

Remove display property from sizeFull #36

merged 2 commits into from
Jul 8, 2016

Conversation

simonsmith
Copy link
Member

Addresses #35

@simonsmith
Correctly surface linting errors
a2b3aec 
Linting errors are caught by `postcss-import` so we need to allow them
through to postcss-reporter
@giuseppeg
Copy link
Member

Does it make sense to remove box-sizing too? Either ways it looks good to me 🚢

@simonsmith
Copy link
Member Author

Does it make sense to remove box-sizing too?

Good question. I did, and the test failed (the element moved outside its parent). I wondered if the utility should make an assumption that the page it lives in has already set border-box. Maybe these days we can assume so?

@giuseppeg
Copy link
Member

In my opinion utilities shouldn't make assumptions.
They should do a single thing (in this case set width to 100%) and have no "side-effects".
If one needs box-sizing he/she can use another utility class or set it globally.

@simonsmith
Copy link
Member Author

Well I guess if we take the box-sizing away, we are assuming that the user handles it. I think that's cool though, and more inline with the doing one thing as you say.

Plus suitcss-base already sets it.

@timkelty
Copy link
Member

timkelty commented Jul 8, 2016

Agreed on all accounts!

@simonsmith
Remove the display property from u-sizeFull
027c3ae 
Setting this display property clashes with other utilities on the
component. This utility should only concern itself with the width and
the border-box (to ensure element does not exceed parent)

Fixes #35
@simonsmith simonsmith force-pushed the refactor-sizefull branch from a9dd9d2 to 027c3ae Compare July 8, 2016 11:20
@simonsmith simonsmith merged commit ca1dad8 into master Jul 8, 2016
@simonsmith simonsmith deleted the refactor-sizefull branch July 8, 2016 11:21
@gerardo-rodriguez gerardo-rodriguez mentioned this pull request Nov 8, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@simonsmith @giuseppeg @timkelty