Add "Overview" object for use on the Article Listing page #1406
Conversation
🦋 Changeset detectedLatest commit: 074105d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| '@cloudfour/patterns': major | ||
| --- | ||
|
|
||
| Renamed the Heading property from `permalink_id` to `id` and allowed you to use `id` when `permalink` is false. |
There was a problem hiding this comment.
This is technically a breaking change, though as far as I can tell we're not usingpermalink_id anywhere yet.
| } | ||
| } | ||
|
|
||
| .topics-header { |
There was a problem hiding this comment.
This prototype CSS was replaced by the new Object CSS.
| .o-overview__header { | ||
| margin-bottom: ms.step(1); | ||
| } | ||
|
|
||
| .o-overview__actions { | ||
| margin-bottom: ms.step(3); | ||
| } | ||
|
|
||
| @supports (display: grid) { |
There was a problem hiding this comment.
Do you think this is necessary? We aren't optimizing for IE, and the content seems perfectly accessible without grid if the spacing's a little off in this section.
I'm open to it, I was just surprised to see these fallbacks and then the margin resets later on for a relatively simple layout object.
There was a problem hiding this comment.
I don't have strong feelings here.
The only change for IE11 was the @supports statement itself. The margins and margin resets were to handle the layout changes between small screens and large screens. I attempted to simplify this an use display: grid with gaps on all screen sizes, but this didn't quite match the prototype design. (On small screens there are two different margin sizes, so it doesn't map to a consistent gap.)
All that said, I'm totally fine removing the supports statement if it's just extra cruft.
There was a problem hiding this comment.
I didn't realize the margin resets were also a responsive concern. In that case this isn't much cruft at all.
| @include fluid.column-gap( | ||
| breakpoint.$s, | ||
| breakpoint.$xl, | ||
| size.$spacing-gap-fluid-min, | ||
| size.$spacing-gap-fluid-max | ||
| ); |
There was a problem hiding this comment.
I'm torn on this. column-gap does seem to be well supported in Grid, though not quite as well supported as grid-column-gap. (We added this mixin for use with the columns property.)
I think it's probably fine, just typing out my initial pause.
There was a problem hiding this comment.
I had initially used grid-column-gap when building out this component but VS Code warned me it was deprecated and that I should use column-gap. The support for column-gap and grid-column-gap seems the same in Safari which is the main modern browser I'd worry about since updates are tied to OS updates.
That said, I don't have strong feelings about it and would be fine swapping over.
src/objects/overview/overview.scss
Outdated
| grid-template-areas: | ||
| 'header header actions' | ||
| 'content content content'; | ||
| grid-template-columns: repeat(3, minmax(max-content, 1fr)); |
There was a problem hiding this comment.
Good question. I'd copied that over from the prototype without thinking about it. This can be simplified.
There was a problem hiding this comment.
I simplified this to grid-template-columns: repeat(3, 1fr);. Let me know if you think a different value would make more sense.
Overview
This PR adds an "Overview" object which is used on the Article Listing pages to show a full list of topics and a search bar. The Article Listings prototype was updated to use this component
Screenshots
Testing
/CC @dromo77