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

Not recognizing unsupported gap property in context of flex box #144

Closed
dakur opened this issue Sep 15, 2021 · 7 comments
Closed

Not recognizing unsupported gap property in context of flex box #144

dakur opened this issue Sep 15, 2021 · 7 comments

Comments

@dakur
Copy link

dakur commented Sep 15, 2021

gap together with display: flex is supported much later than within display: grid.

For example for safari <= 14.1, it should raise a warning on display: flex + gap

@sven-meyer-wetter-com
Copy link

Any updates on this? Run into this today...

It would be very nice to have this detection.

@WebMechanic
Copy link

It would be very nice to have this detection.

@sven-meyer-wetter-com and then what?
if you want to keep the style (likely) you still need to provide a fallback for the flex items using offset margins.

This is best and easily detectable at runtime in the browser anyway and the html/body element can be tagged with that infomation. all you need is a "no-flexgap" selector to compensate.

/* run after DOMContentLoaded, via script module, or deferred script */
function checkFlexGap() {
  const ROOT = document.body; // or document.documentElement
  const flx = ROOT.appendChild(document.createElement('ruby'));
  flx.style.display = 'flex';
  flx.style.flexDirection = 'column';
  flx.style.rowGap = '1px';
  flx.appendChild(document.createElement('div'));
  flx.appendChild(document.createElement('div'));
  if (flx.scrollHeight !== 1) ROOT.classList.add('no-flexgap');
  // console.debug(flx.scrollHeight, flx.style.cssText);
  // flx.style.cssText = '';
  // ROOT.removeChild(flx);
}
checkFlexGap();
.flexed { display:flex; flex-wrap:wrap; gap:1ch; }
html.no-flexgap {
  .flexed > :first-child { .. adjust margins.. }
  .flexed > .items { .. adjust margins.. }
  .flexed > :last-child { .. adjust margins.. }
}

(based on the Modernizr flex gap test)

@sven-meyer-wetter-com
Copy link

@WebMechanic I don't quite understand your answer.

As far as I understood, doisue is all about linting css files against certain browser support. And like @dakur mentioned, Safari <= 14.4 doesn't support display: flex + gap according to the caniuse database. But doiuse don't detect this and does not display a warning.

I don't want to detect anything at runtime or whatever. It would just be nice to have proper warning about the lack of support.

@WebMechanic
Copy link

@sven-meyer-wetter-com I repeat my question: "and then what?"
What would be your plan of action, if the linter did tell you that older Safari versions and others still around do not support this? Something simple any decent front-end developer should know anyway.

There are several CSS properties a script running on the server (in Node) cannot detect w/o access to the actual markup and DOM. This is also true for tools like Stylelint and PostCSS. You'd get false positives from this at best that you need to look after.

I mean you do know it doesn't work for every browser out there. Hence, if you use flexbox and the gap property with it in the same selector to do gap things on the flex items on purpose you should provide a fallback anyway to make your design work and to not be squashed -- _whether some linter tells you so or not. _
It can only provide "proper warning" if both are ued in the very same selector, but miss out on any combinators that may occur at runtime.

If your co-workers don't know: educate them, but don't rely on the output of some arbitrary piece of software that may or may not tell you what's going on especially it it's probably outside it's limited capabilities to provide a useful result.

If you add flex+gap to your style sheet you should by now (several years after this feature has shipped) know that some old and current browsers will not support this particular combination and it is also not detectable at runtime using @supports.
This has always been the case. The CSS WG is aware of the "mistake" around gap, aka flex-gap, and introducing it too late to the party.

You might have a grid/inline-grid/column nested inside a flexbox and the gap might apply to the grid/columns.
So it'd be fine and absolutely legit to use it there. But this information is likely unknown by just looking at some out of context selectors in a style sheet.

The gap might also be introduced by some specific structure or utility class present in the markup unrelated to the context where display: flex was set in the style sheet being linted. It's the final outcome that determines whether gap is valid or not. On its own, gap isn't exotic and a perfectly valid property.

Without the DOM/markup it's hard for a style sheet only parser to understand what's really happening and whether gap in some specific context is supported or not. It would need to rise a notice for every single occurance of gap so you have to check. The next thing for you to ask for is a setting to disable this annoyance because most of the time it'll be for grid and for the few flexbox cases you already added workarounds.

It matters to know at runtime only because wherever you used flex + gap it's on you to also provide some fallback.
The little script I posted could help you with that ot you use a preprocessor mixin to generate the workarounds and fallbacks for you. That's what they're good at.

Have a nice time & viel Spaß.

@clshortfuse
Copy link
Collaborator

#159 attempts to solve this, but I'm not sure it's completely valid. It outputs this:

echo ".bad { display: flex; gap:8px; } | npx doiuse --browsers "chrome >= 66"
<streaming css input>:5:1: gap property for Flexbox not supported by: Chrome (66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,83) (flexbox-gap)

It's fine for rules that include both properties, but it's not 100% foolproof. Because you could split up rules. This outputs no issues:

echo ".flex { display: flex; } .grid { display: grid; } .gap { gap: 8px; }"| npx doiuse --browsers "chrome >= 66"

But an author could write <input class="grid gap">. There's no issue there, but if you write <input class="flex gap">, then it would be an issue.

The fact gap has been overloaded by spec isn't exactly we can control or really monitor. If we decide to add a warning to "gap" then we'd retroactively be breaking perfectly validated CSS.

Checking both is really our best effort to solving the problem, so I'm going to keep the code as it is with the new fix. If properties get overloaded again the future, this "best effort" is probably all we can do for now.

You can help increase your chances of doiuse detecting issues by minifying before passing it, since some minifiers merge rules.

@dakur
Copy link
Author

dakur commented Jul 7, 2023

Thanks for looking into this!

@litera
Copy link

litera commented Feb 16, 2024

@sven-meyer-wetter-com I repeat my question: "and then what?" What would be your plan of action, if the linter did tell you that older Safari versions and others still around do not support this? Something simple any decent front-end developer should know anyway.

One thing is certain. You really don't know what linting is about. At all. Or type checking or whatever else that shows early warning/error to those that develop software. A design time error/warning is a super helpful thing to not release software with bugs. Why do we have type checking and whatnot in our IDEs at all? We as developers are aware of a lot of those things, yet we make mistakes and we don't necessarily know everything. Whether it's coincidental or out of ignorance/lack of experience is a different question, but all of these tools are there to help us write better software out of the box faster and more reliably. So the answer to your "and then what?" is: "Then the developer would address the issue, because they're obviously not aware that they made a mistake. And to make things even more clear, the developer obviously didn't run against this bug, because it works on my machine effect, likely having a browser that has sufficient support of whatever the linter would flip on. I suggest you start writing software in a text editor of your choice that doesn't have any developer-friendly helping hands, and see how far you'll get despite the experience you have with software development.

The fact, that detecting flex+gap issue is an impossible thing just by parsing the code is a different question and I completely agree with you on the rest of your comment, but the premise of and then what is just ridiculously ignorant. The script your provided to detect problematic browsers is of course a sound one. And thank you for that. What one will do with it or if it even helps them to address the problem is a different question.

P.S. There's no need for a reply. You've stated your opinion twice already and there's no need to continue on this matter.

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

5 participants