Fomantic-UI icon indicating copy to clipboard operation
Fomantic-UI copied to clipboard

fix(grid): fix stackable nested grid

Open lubber-de opened this issue 2 years ago • 2 comments

Description

This PR reverts and corrects #1739 for stackable nested grids.

I also made the fixed gutter value from stackable grids on mobile customizable which allows for adjustment to also fix #1907 The intention of a separate/wider gutter on mobile screens for stackable grids was obvioulsy possible better readability decision of SUI, but if now somebody does also want the same close-edge gutter on mobile as on desktop then one can now simply set @stackableMobileGutter to -1rem

Testcase

From #2263 https://jsfiddle.net/lubber/bkrqfnhv/2/

From #1739 https://jsfiddle.net/lubber/fv1hg0jm/12/

Closes

#2263

lubber-de avatar Mar 16 '22 21:03 lubber-de

Hi @lubber-de, thanks for your efforts. I'm not sure if the variable you introduced is a safe solution. Doesn't that apply to all mobile grids now? Same goes for the !important classes. Although I'd love to see them go too, they might disrupt the balance in other scenarios.

I feel this is really an isolated bug inside nested stackable mobile grids. But I will test these changes out of course, to see what happens.

As a sidenote: the double gutter is not a design decision in SUI. See the comments ;)

https://github.com/fomantic/Fomantic-UI/blob/135cfd1f5f29030143ac36968894a52661013820/src/definitions/collections/grid.less#L1721-L1726

This is the original library with the same HTML:

https://jsfiddle.net/db7xuLgo/1/

hugopeek avatar Mar 17 '22 04:03 hugopeek

I'm not sure if the variable you introduced is a safe solution. Doesn't that apply to all mobile grids now?

tbh, i wasn't really convinced myself, yes this would affect all mobile grids, if changed (that's why i kept the default value to the original 0)

Same goes for the !important classes. Although I'd love to see them go too, they might disrupt the balance in other scenarios.

From what i tested, those really were not necessary and removing them just fixed the the issue from #1739 I would appreciate if you could test more deeplyusing your projects 😉

I feel this is really an isolated bug inside nested stackable mobile grids.

Yes, the more i think about that, i also feel this way. The gutter should only be used if the stackable grid isnt nested. Tehre

But I will test these changes out of course, to see what happens.

❤️

As a sidenote: the double gutter is not a design decision in SUI. See the comments ;)

Yes, saw them :) But they meant to not pad when nested. The unnested .ui.stackable.grid 0 gutter seems to be meant for stackable grids which are not nested and a direct child of body (or at least not inside another grid/container/segment) ,i believe. (That's why i guessed the the "fingers covering smartphone sides")

This is the original library with the same HTML:

jsfiddle.net/db7xuLgo/1

That seems to work exactly like my fix in this PR 🙂 https://jsfiddle.net/lubber/bkrqfnhv/2/

lubber-de avatar Mar 17 '22 16:03 lubber-de

The extra mobile gutter is now only used when not nested inside another grid or segment. This fixes all 3 issues now

lubber-de avatar Dec 27 '22 21:12 lubber-de