radix_layouts makes every element use box-sizing: border-box, however, this can cause unexpected problems with lots of CSS that wasn't written with this in mind.

This is fine for Bootstrap-based themes (like Radix), because they can work around these issues. But radix_layouts is meant to work in any theme, including those that aren't Bootstrap!

So, I'd like to propose changes radix_layouts.css to only set box-sizing: border-box on the layout classes. In Radix, you could unset radix_layouts.css and replace it with the full, real Bootstrap CSS (and actually, I think it should work fine even if you don't unset it, but that'll save on the amount of CSS sent over the network).

I'll attach a PoC patch in a moment!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dsnopek’s picture

Status: Active » Needs review
FileSize
1.65 KB

Patch is attached! Please let me know what you think. It works for me in some quick manual testing under Chrome with Responsive Bartik, but I haven't done any really extensive testing.

dsnopek’s picture

FileSize
1.32 KB

Er, it appears that Radix was depending on the WYSIWYG fix I removed in my last patch. Added it back!

klu’s picture

Status: Needs review » Reviewed & tested by the community

While I think border-box is generally a good idea moving forward, for legacy code the across-the-board change will likely cause minor issues with existing modules and themes.

We could tweak css specifically in panopoly (e.g., panopoly-modal.css), but there would likely still be issues in themes and even modules that have existing css built around the older CSS box model (content-box).

Given that Radix Layouts is intended to work with non-Bootstrap themes, I think David's suggestion to limit border-box to the layouts themselves is a good idea.

In my quick tests, they resolve the minor issues David identified AND the Radix theme appears to still work too.

shadcn’s picture

@klu is that patch in #2 RTBC?

klu’s picture

@arshadcn: Yes, I did a quick test of the patch in #2 and it worked for me to solve the issue @dsnopek identified in https://www.drupal.org/node/2191069#comment-9326453 with Responsive Bartik and a non-Bootstrap theme. I did a quick check of the Radix theme, and it appeared to still look the same after applying the patch, but I have less familiarity with the Radix theme, so I haven't done much testing of the Radix theme itself. I assume Radix should be fine though since I see that you're pulling in bootstrap-sass and therefore shouldn't be dependent on radix_layouts.css as @dsnopek mentions in the description. Thanks!

  • arshadcn committed d81e99a on 7.x-3.x authored by dsnopek
    Issue #2373565 by dsnopek: Restrict box-sizing changes to just the...
shadcn’s picture

Status: Reviewed & tested by the community » Fixed

Committed.

  • arshadcn committed d81e99a on 8.x-3.x authored by dsnopek
    Issue #2373565 by dsnopek: Restrict box-sizing changes to just the...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.