There's a strong argument to be made for applying box-sizing: border-box.

  1. It's easier to write fluid layouts, you can do crazy things like width: 80%; padding: 0 10px;
  2. Contrib modules can rely on it being applied, which means they don't need to add inner/wrapper divs on things to have reliable styling.

The downside is this will affect every width set by in core and contrib. We implemented this on the D7 version of Bluecheese and came across a few bugs with contrib module CSS. If contrib modules want a responsive admin interface in D8 they are probably going to have to rewrite all their CSS anyway so this probably isn't that big a deal.

Things left to do:

  • Move the box-sizing "hack" into system.theme.css.
  • Review the new patch again.
  • Take screenshots to make sure we don't break anything.
  • Move into classy?
CommentFileSizeAuthor
#77 apply_box_sizing-2124251-77.patch906 bytesalvar0hurtad0
#72 apply_box_sizing-2124251-72.patch444 bytesmeenakshi.r
#66 apply_box_sizing-2124251-66.patch442 bytesjunaidmasoodi
#62 apply_box_sizing-2124251-62.patch574 bytesjunaidmasoodi
#59 manage_form.png42.08 KBellizard
#59 create article.png128.79 KBellizard
#59 apply_box_sizing-2124251-59.patch547 bytesellizard
#52 Edit contact form.png65.82 KBsamiullah
#52 Manage form display.png81.48 KBsamiullah
#52 Edit contact form.png65.82 KBsamiullah
#52 Create Article.png84.89 KBsamiullah
#52 Content.png169.45 KBsamiullah
#52 Add field.png19.23 KBsamiullah
#52 adminstructureblockmanageseven_breadcrumbs.png10.5 KBsamiullah
#47 issue-box-sizing-2124251-47.patch8.42 KBManjit.Singh
#39 issue-box-sizing-2124251-26.patch8.36 KBjerrylow
#39 Screen Shot 2015-03-30 at 3.53.32 PM.png163.47 KBjerrylow
#39 Screen Shot 2015-03-30 at 3.52.47 PM.png76.71 KBjerrylow
#39 Screen Shot 2015-03-30 at 3.52.36 PM.png364.28 KBjerrylow
#39 Screen Shot 2015-03-30 at 3.52.17 PM.png53.33 KBjerrylow
#39 Screen Shot 2015-03-30 at 3.51.47 PM.png28.28 KBjerrylow
#34 issue-box-sizing-2124251-25.patch8.66 KBjerrylow
#34 2124251-admincss-toolbar.png49.61 KBjerrylow
#34 2124251-admincss-textarea.png49.08 KBjerrylow
#34 2124251-admincss-ckeditor.png133.05 KBjerrylow
#34 2124251-admincss-appearance-not-vertical.png173.52 KBjerrylow
#34 2124251-admincss-appearance.png134.24 KBjerrylow
#27 issue-box-sizing-2124251-24.patch4.43 KBjerrylow
#27 2124251-regression-search-page-move-arrows-fixed.png37.88 KBjerrylow
#27 2124251-regression-search-page-move-arrows.png65.11 KBjerrylow
#27 2124251-regression-text-format-icons.png120.03 KBjerrylow
#27 2124251-regression-throbber.png76.51 KBjerrylow
#27 2124251-vocab-add.png426.38 KBjerrylow
#27 2124251-views-modal.png615.83 KBjerrylow
#27 2124251-views.png530.54 KBjerrylow
#27 2124251-user-edit.png506.35 KBjerrylow
#27 2124251-modules.png508.8 KBjerrylow
#27 2124251-menu-edit.png451.43 KBjerrylow
#27 2124251-manage-fields-edit.png456.68 KBjerrylow
#27 2124251-manage-fields.png421.21 KBjerrylow
#27 2124251-manage-display-configure.png442.98 KBjerrylow
#27 2124251-manage-display.png430.27 KBjerrylow
#27 2124251-files.png388.07 KBjerrylow
#27 2124251-display-mode.png423.24 KBjerrylow
#27 2124251-content-add-type.png566.47 KBjerrylow
#27 2124251-content-add-edit.png648.11 KBjerrylow
#27 2124251-appearance-settings.png685.3 KBjerrylow
#24 issue-box-sizing-2124251-23.patch2.46 KBLewisNyman
#24 Screen Shot 2014-10-24 at 16.12.28.jpg560.81 KBLewisNyman
#12 issue-box-sizing-2124251-12.patch3.24 KBsqndr
#7 issue-box-sizing-2124251-7.patch2.88 KBsqndr
#6 issue-box-sizing-2124251-6.patch317 bytessqndr
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

LewisNyman’s picture

Issue summary: View changes

that

LewisNyman’s picture

Version: 8.x-dev » 9.x-dev
Issue summary: View changes
Issue tags: +frontend, +CSS

No chance of doing this so late in the release cycle

Bojhan’s picture

Is this something we can do in 8.1? If you really want it, we can only do it now (before beta).

sqndr’s picture

Adding this would be a lovely idea. It might be a little to late. If we were to change it now, we need some very good manual testing. There are however good testing frameworks, for taking screenshot of all the pages. Maybe this would be something cool to do at the camp/con/...? Write a patch and ask lots of people to test it. It's a good way as well to introduce them to applying patches and make comments?

* { box-sizing: border-box; } FTW!

LewisNyman’s picture

Version: 9.x-dev » 8.x-dev

Ok, If we have a small window before beta than maybe we can give this a go at frontend united or another camp.

sqndr’s picture

http://www.youtube.com/watch?v=83I9El6C47A (no animated gif this time).

It's something that needs good testing. Let's do this!

sqndr’s picture

Here we go with the first patch ...

sqndr’s picture

Status: Active » Needs review
Issue tags: +FUDK
FileSize
2.88 KB

I've created a new patch. No interdiff, because it didn't make much sense. I removed all of the box-sizing: border-box, including the vendor prefixes. It needs a review, and some proper testing.

sqndr’s picture

Issue summary: View changes
droplet’s picture

Status: Needs review » Needs work

It should includes a rule for before/after generated content.

http://css-tricks.com/inheriting-box-sizing-probably-slightly-better-bes...

sqndr’s picture

Assigned: Unassigned » sqndr
Issue tags: -FUDK

All right.

LewisNyman’s picture

Issue tags: +needs screenshots

If we implement the change in #9 then I am happy to give this a quick manual test + screenshots.

sqndr’s picture

Assigned: sqndr » Unassigned
Status: Needs work » Needs review
FileSize
3.24 KB

Here's a new patch. Meanwhile, all of the Seven css has moved, so I didn't include an interdiff.

Wim Leers’s picture

Is it really safe to lose the vendor prefixes?

-  -webkit-box-sizing: border-box;
-     -moz-box-sizing: border-box;
sqndr’s picture

@Wim

box-sizing: now has native support in those browsers. See the related issue for more information #1664268: Drop some browser specific prefixes.

sqndr’s picture

Issue summary: View changes
sqndr’s picture

Issue summary: View changes
Wim Leers’s picture

Excellent :) I suspected that, but couldn't find confirmation easily on http://caniuse.com. Thanks!

LewisNyman’s picture

Issue tags: +Needs manual testing

All we need now is for someone to double check a load of pages in the Seven theme to make sure we haven't broken anything visually. View's UI is a good place to check.

sqndr’s picture

If someone wants to start with this, go ahead. Otherwise I'll try to write an automated test for this.

sqndr’s picture

We need a review for this issue :)

LewisNyman’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

Aw sorry I needs a reroll. I will reroll and test

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 12: issue-box-sizing-2124251-12.patch, failed testing.

LewisNyman’s picture

Component: Seven theme » CSS
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
560.81 KB
2.46 KB

I found a regression on table drag handles:

I also found box-sizing declaration in the following admin.css files:

core/modules/block/css/block.admin.css
core/modules/ckeditor/css/ckeditor.admin.css
core/modules/node/css/node.module.css
core/modules/system/css/system.admin.css
core/modules/views_ui/css/views_ui.admin.css

That is appears in so many places is a good sign in terms of any UI regressions, I feel like we should remove these values and move the declaration into system.admin.css like we did in #2298821: Move generic layout styling into system.admin.css

tim.plunkett’s picture

Issue tags: - +Needs screenshots

Fixing tags.

jerrylow’s picture

Assigned: Unassigned » jerrylow

Verifying and grabbing some screens.

jerrylow’s picture

+++ b/core/themes/seven/css/components/tabs.css
@@ -36,7 +36,6 @@
   padding: 9px 2em 7px 1em;

There's a new /*LTR*/ added to this line now. I'm adding it to the updated patch here. I've manually gone through all the pages and settings. Attaching a bunch of screens of all the pages working (desktop testing only - thus far).

Here are some other regressions I've found:

Besides the drag icon that Lewis have found I've noticed these drag icons in the search page settings is not affected:

It does break when the other arrows are fixed, but if we move * { box-sizing: border-box; } to system.admin.css it should fix it - here's a screenshot of it inline:

The arrows are centered but looks like the text is not.

Updated patch for the tabs.css change as well as fixing the regression styles. Haven't moved the admin settings to system.admin.css yet.

brahmjeet789’s picture

Status: Needs review » Reviewed & tested by the community
brahmjeet789’s picture

Status: Reviewed & tested by the community » Needs review
sqndr’s picture

What work still has to be done for this issue?

LewisNyman’s picture

@sqndr I think we just need to verify the regressions in #27 are fixed. And maybe another click around just to make sure we haven't missed anything else

sqndr’s picture

Haven't moved the admin settings to system.admin.css yet.

Does this mean there's still work to do as well?

LewisNyman’s picture

Status: Needs review » Needs work
Issue tags: -Needs manual testing, -Needs screenshots

It makes sense to move it to system.admin.css.

jerrylow’s picture

jerrylow’s picture

Status: Needs work » Needs review
nitishchopra’s picture

The above patch looked fine for me.

idebr’s picture

Status: Needs review » Needs work

With manual testing I found a few changes that should be fixed:

  1. /admin/structure/types/manage/article/form-display
    Table drag handle width is smaller
  2. /admin/reports/dblog
    Warning/error icon column is smaller
  3. /admin/appearance
    Screenshots from themes are smaller
  4. /admin/config/content/formats/manage/basic_html
    Buttons in the CKEditor are smaller
  5. /admin/config/content/formats/manage/basic_html
    Vertical tabs summary now has a grey border on the right
sqndr’s picture

@jerrylow: Any progress here?

jerrylow’s picture

@idebr @sqndr - I just did a fresh rebuild, applied the patch and clear cached. Can't see any of the issues mentioned about. Screenshots of the pages attached. I've updated the patch since dev has changed.

jerrylow’s picture

Status: Needs work » Needs review
sqndr’s picture

Issue summary: View changes
Karmen’s picture

Status: Needs review » Reviewed & tested by the community

I've tested these paths and works for me.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

AFAICS this is only changing the box model on admin pages but it is changing CSS that might not be on an admin page - for example dialog and quickedit.

LewisNyman’s picture

That's true, I think we would have to be consistent here across all themes if modules are relying on this when they set width. Either we don't set this at all and let it be set on a per-component level, or we set this globally across all themes. We could put this in system.theme.css, with an idea of moving it into Classy when the time comes.

sqndr’s picture

or we set this globally across all themes

+1 for that!

(Bootstrap and Foundation use this globally as well, instead of on a per component base).

Manjit.Singh’s picture

Issue tags: +Needs reroll
Manjit.Singh’s picture

rerolling a patch

Manjit.Singh’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll

Manjit.Singh’s picture

Issue tags: +india, +SprintWeekend2015
samiullah’s picture

Assigned: jerrylow » samiullah
samiullah’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
10.5 KB
19.23 KB
169.45 KB
84.89 KB
65.82 KB
81.48 KB
65.82 KB

After applying latest patch above "box-sizing: border-box" is getting applied globally.
Manually tested on the following screens:

  • admin/structure/contact/manage/feedback/fields/add-field
  • admin/structure/contact/manage/feedback
  • admin/structure/contact/manage/feedback/form-display
  • admin/structure/contact/manage/feedback/display
  • admin/structure/views/view/content
  • /node/add/article
alexpott’s picture

#43, #44, #45 don't look as though they've been addressed by the most recent patch.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
sqndr’s picture

@Lewis:

We could put this in system.theme.css, with an idea of moving it into Classy when the time comes.

What do you mean by that?

sqndr’s picture

Issue summary: View changes
sqndr’s picture

Issue tags: +Novice
deepakaryan1988’s picture

Issue tags: -india, -SprintWeekend2015

Removing sprint weekend tag!!
As suggested by @YesCT

ellizard’s picture

Status: Needs work » Needs review
Issue tags: -ie8, -frontend, -Novice
FileSize
547 bytes
128.79 KB
42.08 KB

Apply 'box-sizing: border-box' globally to all element

Status: Needs review » Needs work

The last submitted patch, 59: apply_box_sizing-2124251-59.patch, failed testing.

junaidmasoodi’s picture

Assigned: samiullah » junaidmasoodi
junaidmasoodi’s picture

Applying box-sizing: border-box; globally

This reset gives us more flexibility than its predecessors — we can use content-box or padding-box (where supported) at will, without worrying about a universal selector overriding our CSS.

junaidmasoodi’s picture

Status: Needs work » Needs review
sqndr’s picture

+++ b/core/modules/system/css/system.theme.css
@@ -4,6 +4,21 @@
+  -webkit-box-sizing: border-box;
+  -moz-box-sizing: border-box;
...
+  -webkit-box-sizing: inherit;
+  -moz-box-sizing: inherit;

I don't think we need to add vendor prefixes again. Browser support this native now.

junaidmasoodi’s picture

Status: Needs review » Needs work
junaidmasoodi’s picture

Status: Needs work » Needs review
FileSize
442 bytes

Patch re-submitted with removal of vendor prefixes

Manjit.Singh’s picture

Assigned: junaidmasoodi » Unassigned
Status: Needs review » Reviewed & tested by the community

looks good to me !! changing this to RTBC :)

sqndr’s picture

Status: Reviewed & tested by the community » Needs review

Thanks everyone for working in on this issue. When look at the latest patch I have some concerns …

#62 (and #66) is basically the work I did all the way back in #12. We had to work on this issue to find regressions and fix those. We also had to move all the box-sizing: border-box; properties from all other elements, since we now add it globally. Also, we had to add content-box; to elements that require a different box-sizing calculation.

Since we're now all the way back to were we started, this feels odd to me. I think we need another review and good screenshots to show that everything is fixed now. I'm adding Needs Review back.

LewisNyman’s picture

Issue tags: +frontend, +Needs screenshots
Manjit.Singh’s picture

Status: Needs review » Needs work
+++ b/core/modules/system/css/system.theme.css
@@ -4,6 +4,16 @@
+.html {
+  box-szing: border-box;
+}

spells of box-sizing is not correct in #66 patch.

sqndr’s picture

+++ b/core/modules/system/css/system.theme.css
@@ -4,6 +4,16 @@
+ *Universal Box Sizing with Inheritance

And there should be a space after the * here.

meenakshi.r’s picture

Status: Needs work » Needs review
FileSize
444 bytes

Changes as suggested #70 & #71. Please verify.

sqndr’s picture

Status: Needs review » Needs work

Please read #68 and provide screenshots to verify the regressions are fixed.

meenakshi.r’s picture

Assigned: Unassigned » meenakshi.r
Issue tags: +Needs reroll

Shamsher_Alam’s picture

Assigned: meenakshi.r » Shamsher_Alam
alvar0hurtad0’s picture

Assigned: Shamsher_Alam » Unassigned
Status: Needs work » Needs review
FileSize
906 bytes

@Shamsher_Alam still working?

this is my approach, I've created a new css in classy because this issue #2566597: [Mega patch] Move system *.theme.css files to Classy

sqndr’s picture

I don't think this can still go in, because this will drastically change the styling on all pages and all css elements (since the use of *). I feel like this should be postponed.

LewisNyman’s picture

Version: 8.0.x-dev » 8.1.x-dev
Status: Needs review » Postponed

You are right, it also feels like we need to reduce the scope of this as we make this change to Classy and Stable.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.