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

[Grid] Compiled CSS for child .rows not as expected when wrapped in ID/Class #9225

Closed
coogie opened this issue Oct 3, 2016 · 6 comments
Closed

Comments

@coogie
Copy link

coogie commented Oct 3, 2016

Hi all.

I have a situation in which I'm trying to encapsulate the Foundation Grid styles behind an ID - we have a product that can be injected into other websites and I'm trying to ensure that any styles we need don't bleed out and interfere with the parent site.

However, I've got small issue when I wrap the Foundation Sites grid in a class or ID in that the output CSS for the child .row classes is not as expected. The wrapper ID/Class is placed in some unexpected places.

How can we reproduce this bug?
Consider the following setup in my-app.scss:

@import 'foundation-sites/scss/util/util';
@import 'foundation-sites/scss/global';
@import 'foundation-sites/scss/grid/grid';

#WRAP {
    @include foundation-flex-grid();
}

What did you expect to happen?
I expected the Foundation Sites grid styles to have the additional specificity of my ID/Class.
E.g.: #WRAP .row .row

#WRAP .row {
  max-width: 75rem;
  margin-left: auto;
  margin-right: auto;
  display: flex;
  flex-flow: row wrap;
}
#WRAP .row .row {
  max-width: none;
  margin-left: -0.625rem;
  margin-right: -0.625rem;
}
@media screen and (min-width: 40em) {
  #WRAP .row .row {
    margin-left: -0.9375rem;
    margin-right: -0.9375rem;
  }
}

[...]

#WRAP .column.row.row, #WRAP .row.row.columns {
  float: none;
  display: block;
}
#WRAP .row .column.row.row, #WRAP .row .row.row.columns {
  padding-left: 0;
  padding-right: 0;
  margin-left: 0;
  margin-right: 0;
}

#WRAP .small-1 {
  flex: 0 0 8.33333%;
  max-width: 8.33333%;
}

[...]

What happened instead?
The parent ID/Class is added in some strange positions in relation to .row.
E.g.: #WRAP .row #WRAP .row

#WRAP .row {
  max-width: 75rem;
  margin-left: auto;
  margin-right: auto;
  display: flex;
  flex-flow: row wrap;
}
#WRAP .row #WRAP .row {
  max-width: none;
  margin-left: -0.625rem;
  margin-right: -0.625rem;
}
@media screen and (min-width: 40em) {
  #WRAP .row #WRAP .row {
    margin-left: -0.9375rem;
    margin-right: -0.9375rem;
  }
}

[...]

#WRAP .column.row.row, #WRAP .row.row.columns {
  float: none;
  display: block;
}
.row #WRAP .column.row.row, .row #WRAP .row.row.columns {
  padding-left: 0;
  padding-right: 0;
  margin-left: 0;
  margin-right: 0;
}
#WRAP .small-1 {
  flex: 0 0 8.33333%;
  max-width: 8.33333%;
}

[...]
@coogie coogie changed the title [Grid] Compiles CSS for child .rows not as expected when wrapped in ID/Class [Grid] Compiled CSS for child .rows not as expected when wrapped in ID/Class Oct 3, 2016
@brettsmason
Copy link
Contributor

@coogie Thanks for the report.
I've ran a test myself and I see what you mean. I have identified the .row issue, its this line: https://github.com/zurb/foundation-sites/blob/develop/scss/grid/_flex-grid.scss#L158

Changing & & to & .row solved this particular issue for me.

Were there any other specific issues you found in the generated CSS? If you could let me know and I'll put a PR together to fix this.

@brettsmason
Copy link
Contributor

https://github.com/zurb/foundation-sites/blob/develop/scss/grid/_flex-grid.scss#L242 this also looks like it needs changing to & .row.#{$-zf-size}-unstack {

@coogie
Copy link
Author

coogie commented Oct 5, 2016

Thanks @brettsmason,

I can confirm that this is happening on the traditional float-based grid, too, with the following rules:

#WRAP .row #WRAP .row

[…]

#WRAP .row #WRAP .row.collapse 

[…]

.row #WRAP .column.row.row, .row #WRAP .row.row.columns

[…]

#WRAP .large-collapse .row, .expanded.row #WRAP .large-collapse.row

As for whether it's happening with items such as components, I can't say - I'm only using the grid in this way.

Also, from the looks of it, the unstack classes are compiling as expected so that may not actually need to change:

#WRAP .row.large-unstack > .column, #WRAP .row.large-unstack > .columns {
  flex: 0 0 100%;
}
@media screen and (min-width: 64em) {
  #WRAP .row.large-unstack > .column, #WRAP .row.large-unstack > .columns {
    flex: 1 1 0px;
  }
}

@brettsmason
Copy link
Contributor

brettsmason commented Oct 6, 2016

@kball @andycochran I've been looking at fixing this, but I've noticed some of the CSS/Sass looks a bit odd.
If you take the example above (ignoring #wrap), this doesn't make any sense to me:
.row #WRAP .column.row.row, .row #WRAP .row.row.columns

You wouldn't have 2 .row declarations on the same element. I think there may be a larger issue here but would appreciate a 2nd pair of eyes on it.

Edit: Having re-read the comments in the code it appears this is needed

@brettsmason
Copy link
Contributor

@coogie I have added a PR addressing this.
If possible would could you double check this solves your problems? I have double checked the flex grid and normal grid output and this should now be correct.

samuelmc added a commit to samuelmc/foundation-sites that referenced this issue Oct 7, 2016
* Make off canvas work no matter where you are on the page and avoids having to jump to the top.

* Error in documentation

Native element sass usage: @import replaced by @include, which it should be

* Update progress-bar.md

replace import by include

* Add missing overflow style

Add in missing `overflow-y: auto` to `.off-canvas-content` to allow for vertical scrolling.

* Fix disabled button hover and focus state background color

* Fix dropdown alternative anchor selection

* Add settings for the hover color for thead and tfoot

* Added !global back to $grid-column-count

* Round rgb color values otherwise it will always be black

* Fix iOS bug where opening a modal would cause the page to jump to top.

* custom button hover lightness

* custom button hover lightness settings

* configurable pagination current page number on mobile

* add default settings value for $pagination-mobile-current-item

* foundation#9015 option to close drop menu on leaf anchor tag click

* check if cb is a function

* check if cb is a function

* orbit: add event beforeslidechange

I use this event to smoothly resize the orbit container for carousels with images of different heights

* added event docblock for docs autogen

* Enhanced functionality of Accordion.

Click listener now only calls toggle(), which will then work the tab content up/down out on its own.

* Added unit tests for Accordion.

* Included feedback from PR.

* Removed unused variable and check if target is active before executing up().

* Included done callback in describe call properly.

* Added unit tests for Accordion Menu.

* Adjusted test case for up().

* Fixed wrong reference to triggerClass option.

* Added unit tests for Tooltip.

* Remove version specification on composer.json as per issue foundation#9144

* Add "alert" as required map value to $foundation-palette

Undefined "alert" color in $foundation-palette breaks SCSS compilation. Documentation incorrectly identifies "primary" as the only required value in the user redefined map.

* Fixed toggling of ARIA attributes.

* Added unit tests for Drilldown.

* Fix last child to float to the global left direction.

* Fix for flex grid not working correctly when applied to an element (namespaced).
Part fix for foundation#9225

* Fixes the default grid to work when added to a container (namespaced).
Fully fixes foundation#9225

* Fix incorrect flex grid output.
The output should of been `.row .column.row.row, .row .row.row.columns`

* Remove not needed `&` added during testing to find the problem.

* Fixed missing newline at end of file.

* Fix foundation#9230 - remove code duplication

foundation#9230

Remove `.menu-icon.dark` in `scss/components/_title-bar.scss`

* Get rid of JS scroll behavior on reveal taking advantage of change in scss
@coogie
Copy link
Author

coogie commented Oct 7, 2016

@brettsmason it looks as though that has solved the issue!
Thanks for looking into it, and thanks for the speedy resolution. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants