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

Setting visible on a Grid.Unit sets it to display:flex #13

Open
lauriejones opened this issue Jun 22, 2018 · 5 comments
Open

Setting visible on a Grid.Unit sets it to display:flex #13

lauriejones opened this issue Jun 22, 2018 · 5 comments

Comments

@lauriejones
Copy link
Collaborator

lauriejones commented Jun 22, 2018

This forces the grid unit to be flex, and then the <Cell> behaves like an inline-block element and is not longer full width.

<Grid>
  <Grid.Unit visible><Cell>always visible</Cell></Grid.Unit>
</Grid>

https://github.com/jameslnewell/styled-components-grid/blob/master/src/mixins/gridUnit.js#L48

I am still using version 1 but I think it looks like it would still happen in v2.

@lauriejones lauriejones changed the title Setting visible on a Grid.Unit sets it to display:flex Setting visible on a Grid.Unit sets it to display:flex Jun 22, 2018
@jameslnewell
Copy link
Owner

Yeah 😬any ideas how to fix so it'll work for cases like <Grid.Unit visible={{tablet: false, desktop: true}}/>?

@jameslnewell
Copy link
Owner

jameslnewell commented Jun 25, 2018

Will display: unset work? https://codepen.io/jameslnewell/pen/oyMoob

@lauriejones
Copy link
Collaborator Author

Does work – but not for IE11. Do you care about that?

Also, are you supporting Grid.Units being display:flex; for those times where you need to make a child of a grid unit full height?

Because if not, can't you just set visible to the default for a div, display: block;?

@jameslnewell
Copy link
Owner

jameslnewell commented Jun 26, 2018

Does work – but not for IE11. Do you care about that?

Not personally no... do you? 😁

Also, are you supporting Grid.Units being display:flex; for those times where you need to make a child of a grid unit full height?

There's no prop to switch a grid unit to display: flex, but there's nothing stopping someone using the mixins and setting display to flex. And in this case even unset would break whatever they had right?

Suggestions?

@lauriejones
Copy link
Collaborator Author

lauriejones commented Jun 26, 2018

You could have a bunch of min/max queries for it? And only set display: none for the breakpoints required.

So for this example<Grid.Unit visible={{tablet: false, desktop: true}}/> you'd have:

@media (min-width: 'tablet') and (max-width: 'desktop') {
  display: none;
}

So we don't set anything for the breakpoints where visible !== false.

It's going against mobile-first principles, but might work 😅

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

2 participants