grid-visual not behaving as expected

[MikeHarrison]

Hi,

I just downloaded the latest Beta and am seeing an issue with grid-visual - it was working on the previous beta, now the background isn't displaying properly. I may have an issue with my syntax, or it may be something that is being worked on, in which case just let me know.

Browser
Chrome Version 55.0.2883.95 (64-bit) on macOS Sierra 10.12.2

Code

.container {
  @include grid-container;
  @include grid-visual;
  max-width: 900px;
  background-color: rgba(pink,0.3);
  height: 400px;
  margin: 0 auto;
}

Result

[whmii]

@MikeHarrison Chrome recently made changes that broke the 'grid-visual' mixin. Originally it displayed correctly and the mixin itself is functioning correctly. The 'grid-visual' output is very complex and I believe that chrome is basically going... "seems like a lot of work, I'm not going to render that"

If you take a look at the page in safari it will render correctly. I have a couple of thoughts/options here.

1. Leave it be. If a particular browser decides not to render this feature, it may not be worth addressing, especially for a debugging feature.
2. Change the mixin to use a more simplified look that may be supported in more browsers.

I'm honestly open to either so let me know your thoughts.

[MikeHarrison]

Thanks for the quick reply.

I have now updated my MacOS (to 10.12.3), which in turn has updated my browsers:

Chrome: 56.0.2924.87
Safari: 10.0.3

Chrome is now rendering the visual grid correctly, and Safari is having a freakout. Such is life.

I take your point about it being a debugging feature, my concern would be that you would have to either have a disclaimer in the docs that it may or may not work in the user's chosen browser, or field issues from people who are trying to get it to work in a browser that isn't rendering it correctly.

I love the look of it, and am finding it much better for working over as it distracts less from the layout then the previous incarnation with coloured bars.

So my suggestion would perhaps be an additional argument to the mixin, something like solid, which would flip the visual grid to using the old behaviour, using the color argument to set the fill colour. Then have in the docs that if the default isn't working in your browser, to try the solid (or whatever) version. Best of both worlds, but perhaps additional complexity you would want to avoid?

Would be happy to put in a PR if you think this might be a good route

[whmii]

@MikeHarrison I think your intuition of doing something that actually works as is correct 😆. For now I think the best course of action is to change the look of grid-visual() to be bars instead of the fancy grid nonsense I made. check out 00a80d5 as I believe I implemented color bars before creating the current style.

One other note I might have is that the current implementation has a few extra lines including the top and bottom (see: https://github.com/thoughtbot/neat/blob/master/core/neat/mixins/_grid-visual.scss#L78). This was just done for style points but removing it may resolve the issue. It would be good to try removing this before resorting to other measures.

[whmii]

If you want to post a pr that would be great. fortunately I feel as though changing the display properties of grid-visual is not a 'breaking change', aka I think we can change the look and feel between minor versions and that's fine. If browsers update and are able to handle the current implementation we can always switch it back whenever. 👍

[MikeHarrison]

Removing the top and bottom lines don't fix it unfortunately.

OK I concur - back to the bars, though you get to keep the style points for the lines as they looked much cooler 💯

[whmii]

Sweet 🍬! I think this might be the last feature change before the release!

https://youtu.be/XaRkVnysuMU

[MikeHarrison]

Rolling back to your bars version fixed the issue in all browsers except IE11 - that didn't like me doing a window resize. But I think you could count on the fingers of one foot how many people will be debugging like that in IE11.

Don't have time to work through a PR at the moment, will leave it to the pros 😄