propublica / column-setter Goto Github PK

Right now, josef lets you define a grid globally, and reference those layouts whenever @include grid(classname); is invoked.

What I’d love to be able to do is to define a map of properties, and reference that with the grid() function. Just for example, some pseudo-sass:

$grid-mast: (
    "mar": 68,
    "col": 68,
    "gut": 26,
    …
    "breakpoints": (
        "small": (cols:  4, min:  0,   margin: $pad),
        "med": (cols:  6, min: 30em, margin: $gut),
        "large": (cols:  8, min: 40em)
    )
);

$grid-content: (
    "mar": 68,
    "col": 68,
    "gut": 26,
    …
    "breakpoints": (
        "xs": (cols:  4, min:  0,   margin: $pad),
        "sm": (cols:  6, min: 40em, margin: $gut),
        "md": (cols:  8, min: 60em),
        "lg": (cols: 12, min: 78em),
        "xl": (cols: 12, min: 95em)
    )
);

@include grid( .mast-content, $grid-mast );

@include grid( .page-content, $grid-content );

This’d be a huge help for regions that might have unique breakpoint needs, and/or simpler (or more complex) grids.

Allow developers to turn the grid overlay on and off in the browser by toggling the G key.

There doesn’t seem to be a way for the grid module’s children to know the grid module’s width relative to the viewport, which would be necessary to make this work.

Right now, the grid function accepts a class name for its first argument:

@include grid(content);

, which applies a grid to anything with a .content class.

I think it’d be great if this argument could be a full CSS selector. So, for example, these would all be valid:

@include grid( ".content" );
@include grid( "#www-propublica-org .content" );
@include grid( "#www-propublica-org-IL .content" );

I think this, coupled with #11, could make for a nice bump in flexibility of how/when josef’s applied.

Often, it's convenient to set the padding in ems/rems, so it relates to the text size rather than the viewport width. Because border-box is assumed throughout this library (I think), allowing the user to provide padding with an ems/rems unit shouldn't effect any width calculations. In fact, I think supporting this would be as simple as returning the value (with its unit) on colspan(p, ...) calls, and removing the unitless() check in the bp-validator function. Is this something you all would be interested in adding?

It'd be nice for people depending on this library to make that dependency explicit and have an easy way to pull in any updates. The natural way for that would seem to be publishing this as an npm package.

Allow the user to optionally customize the z-index of the column overlay, like so:

$overlay-z-index: 100;

I'm starting an early attempt to upgrade our implementation of Column Setter to v2. I tried copying in the new versions without changing anything else just to see what would happen:

Deprecated / for division

Sass doesn't allow using / to divide anymore, but they do have some good resources, including an automatic upgrader.

npm install -g sass-migrator
sass-migrator division **/*.scss

As far as I can tell, running these commands on Column Setter works fine.

Invalid max-width

I've previously had $max-width: null; in my settings, as shown in the example. In v2, this causes this error:

Warning: $max-width's value ('') in _column-settings.scss is invalid. It must be a number with a unit. Learn more: https://github.com/propublica/column-setter

When I change it to $max-width: 80em; the error goes away and the CSS compiles without complaint. I haven't gone through to see what design implications this might have for us, but I thought you might want to change the docs if $max-width: null; is no longer supported.

Eliminates the danger of conflicting with other Sass libraries.

Since grid settings are all currently integers ($mar: 68; $col: 68; $gut: 26;), but $max-width is defined as a string (e.g., $max-width: 1680px;), it’s a bit tricky to do Sass calculations outside of josef.

For example, $mar / $max-width * 100% results in a Sass compilation error, as it’s asking Sass to divide 68 / 1680px. (Error: 4.04762%/px isn't a valid CSS value.) Manually converting $mar to a string does the trick, though:

( ( $mar + 0px ) / $max-width ) * 100%

E.g. “ten” or “10px” instead of “10”.

If you use the full-widthfunction on an object at a certain breakpoint but don’t want it to be full width at later breakpoints, you have to override the width, margin-left, and margin-right values generated by full-width. Since full-width is a mixin, the code it generates isn’t immediately apparent, and you might not know what values need to be overridden. Should there be a function that zeros out those values?

Something that came up when working on the “Get Involved” module: when padding is applied to a container, it adds quite a bit of complexity to inner colspan() calculations. Just e.g.,

.promo-get-involved .brief-thumb {
    float: left;
    width: ( colspan( 1, 4 ) - colspan( p, 4 ) ) + colspan( p, 12 );
    @include breakpoint-range(sm, lg) {
        float: right;
        margin-left: $scale-5;
        width: 25%;
    }
}

One possible solution for this—though probably not the only one!—would be to allow the two colspan() arguments to accept expressions, like so

colspan( 1 - g, 4 - p )

I realize that, uh, would probably get pretty complicated pretty quickly, but thought I’d mention it as a possibility.

Time to drop the "Josef" code name and open source. Will be called Column Setter going forward, two words with a space between them in prose, hyphenated in code (ex. column-setter).

To do:

• Change repo name in GitHub (@Stunbox)
• Update name and function references in code (@robweychert)
• Review and update README (@stuntbox and @robweychert)
• Add description, topics to repo
• Add license
• Go to GitHub Options panel, make repo public

Rather than having three separate breakpoint mixins (breakpoint-min, breakpoint-max, breakpoint-range), would it make more sense to consolidate them into one breakpoint mixin, with an initial argument of min, max, or range (e.g. breakpoint(min, xl))? It would be a more DRY approach, but error handling would get a bit thorny.