Unexpected wrapper width coming from the DS

[natalia-fitzgerald]

Description

As a part of our work on the DSR footer component we noticed a layout setting of 1200px (plus 30px of left and right padding) for a total of 1260px width. In order to accomplish the 1170px width with 30px left and right padding @virginiacc did an "override DS wrapper width." This got me curious as to why the DS is defining a wrapper width that isn't the standard 1170px = 30 px of padding on each side for a total of 1230px wide.

The purpose of this ticket is to allow for an asynchronous discussion as to the current state and how we should move forward.

Discovery

@flacoman91 provided the following information after taking a closer look

https://github.com/cfpb/design-system/blob/main/packages/cfpb-design-system/src/components/cfpb-layout/layout.scss#L300

• $base-font-size-px: 16px;
• $bp-sm-min: 601px;
• $bp-med-min: 901px;
• $grid-gutter-width: 30px;
• $grid-wrapper-width: 1230px

it evaluates to:

@media only screen and (min-width: 56.3125em) {
 .wrapper {
  max-width: 1200px;
  padding-right: 15px;
  padding-left: 15px;
  margin: 0 auto;
  clear: both;
}
}
.wrapper--match-content {
  padding-left: 15px;
  padding-right: 15px;
}
@media only screen and (min-width: 37.5625em) {
  .wrapper--match-content {
    padding-left: 30px;
    padding-right: 30px;
    max-width: 1200px;
  }
}

It almost looks like thats a bug in DS
this line here:
https://github.com/cfpb/design-system/blob/main/packages/cfpb-design-system/src/components/cfpb-layout/layout.scss#L300
max-width: $grid-wrapper-width - $grid-gutter-width;

@anselmbradford provided this information after doing some digging.

In that code,$grid-wrapper-width is 1230px.$grid-gutter-width is 30px. .wrapper--match-content has a max-width of 1170px at tablet size and above wrapper--match-content with that specific max-width rule has existed since Capital Framework cf-layout and was added by Jimmy in cfpb/cf-layout#23 and eventually made its way into the DS from there.

In cfgov, there is an "enhancements" file for adjustments and additions to DS CSS classes that aren't working right in cfgov, but haven't yet been vetted for change in the DS itself.

cfgov has an "enhancement" for wrapper--match-content here https://github.com/cfpb/consumerfinance.gov/blob/main/cfgov/unprocessed/css/enhancements/layout.scss#L7

…that changes the max-width from $grid_wrapper-width - $grid_gutter-width to $grid_wrapper-width - ($grid-gutter-width * 2) (so changes 1230 - 30, to 1230 - 60)

The DS doc site copied that "enhancement" over in this file https://github.com/cfpb/design-system/blob/main/docs/assets/css/layout.scss, but the original still is in the distributed npm module. (you can see the comment about it coming from cfgov enhancements here (https://github.com/cfpb/design-system/blob/8a2e97e1bcca99649c8aa1fa9ae2816c988585af/docs/assets/css/layout.scss#L3)

Unfortunately, the banners (https://cfpb.github.io/design-system/components/banner-notification) do use this class, even though it hasn't settled out of this "enhancements" setup.

For the DSR, it looks like the same setup was used, and the .wrapper--match-content CSS class was copied over to this file (https://github.com/cfpb/design-system-react/blob/main/src/assets/styles/_shared.scss#L64)

So, in the code you posted, I see the code comment "// Override DS wrapper match-content width to 1170px content with 30px gutters.", but that was added by Richard in #542, and is saying that snippet (copied from cfgov "enhancements" is overriding what is in the DS npm package—the same as is done in cfgov, the DS, and the DSR). Virginia's PR adds the !important to that, which makes it override max-width in any other definitions of that class.

However, I don't see why that is necessary (that'd be a good question for Virginia). Maybe the order the CSS loads in the DSR is different, so that the layout enhancement CSS loads first, and the DS code loads second, so it ends up with the original wrapper--match-content from the DS npm package.

Additionally, I mentioned the only component in the DS that directly uses this class is the banners, and in the DSR, that entire wrapper--match-content class is redefined here https://github.com/cfpb/design-system-react/blob/main/src/components/Banner/banner.scss#L57

What should probably happen is the DS npm code should be updated with the "enhancement" and then the "enhancement" snippet should be removed from cfgov, the DS doc site, and the DSR, and then the DSR banner should be scrutinized for what it is doing special with wrapper--match-content.

Speculating as to why this was not already done, CCDB used to have a copy of the header/footer with minor modifications, and maybe in CCDB it used the original wrapper--match-content sizing and not the enhanced version, so for that reason the enhancement was left. But that's speculation.

@virginiacc provided this information

It looks like cf.gov has wrapper rule for the 1170px: https://github.com/cfpb/consumerfinance.gov/blob/f372c54575459844a99e9934f04bd46614e65c00/cfgov/unprocessed/css/enhancements/layout.scss#L7-L12

It looks like in the DSR we're overriding this rule from the DS: https://github.com/cfpb/design-system/blob/9afd8994a08bddf97ca349d9c41a8e388f473aaa/packages/cfpb-design-system/src/components/cfpb-layout/layout.scss#L314

So maybe that should be updated in the DS?

@virginiacc @flacoman91 @anselmbradford @billhimmelsbach

[natalia-fitzgerald]

Let's discuss at the next DSR biweekly and clarify next steps:

• Update the CFPB Design System npm code with the "enhancement"
• Remove the "enhancement" snippet from cfgov, the DS doc site, and the DSR
• Look into why the DSR banner-notification has its own wrapper--match-content rule and whether it can also be removed