Skip to content
This repository was archived by the owner on Jun 26, 2018. It is now read-only.

DP-5343 Callout Grid #578

Open
gender404 wants to merge 9 commits into
base: dev
Choose a base branch
from
Open

DP-5343 Callout Grid #578

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8e3761f
DP-3232 & DP-1369 Key Actions and Key Agencies showing multiple times…
Aug 31, 2017
0c84e79
DP-5343 - Comp Heading - removed extra whitespace in content
Aug 31, 2017
3c45abe
DP-5343 - Key Actions - Adding id and option to hide the heading
Aug 31, 2017
80202b6
DP-5343 - Action Finder - adding Key Actions and Comp Heading.
Aug 31, 2017
78fe66b
DP-5343 - Action Finder - Removed filter class
Aug 31, 2017
a6cb9d3
Merge remote-tracking branch 'origin/dev' into DP-5343-CalloutGridsv2
Sep 20, 2017
df7ce3d
Merge branch 'dev' into DP-5343-CalloutGridsv2
Sep 20, 2017
ce9cc26
PR review updates
Sep 28, 2017
d192c42
DP-5343 - Key Actions - fixing ID logic
Sep 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions styleguide/source/_patterns/01-atoms/04-headings/comp-heading.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{
"compHeading": {
"title": "Title text",
"title": "Comp Heading",
"level": "",
"color": "",
"id": "GUID9827924",
"centered": ""
"id": "GUID285724897",
"centered": false
}
}
4 changes: 2 additions & 2 deletions styleguide/source/_patterns/01-atoms/04-headings/comp-heading~as-sidebar-heading.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{
"compHeading": {
"title": "Employment",
"title": "Comp Heading",
"sub": true,
"color": "",
"id": "employment",
"id": "GUID28357207",
"centered": false,
"sidebar": true
}
Expand Down
4 changes: 2 additions & 2 deletions styleguide/source/_patterns/01-atoms/04-headings/comp-heading~centered.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{
"compHeading": {
"title": "Title text",
"title": "Comp Heading",
"sub": false,
"color": "",
"id": "",
"id": "GUID248572057",
"centered": true
}
}
4 changes: 2 additions & 2 deletions styleguide/source/_patterns/01-atoms/04-headings/comp-heading~subheading.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{
"compHeading": {
"title": "Title text",
"title": "Comp Heading",
"level": 3,
"color": "",
"id": "",
"centered": ""
"centered": false
}
}
2 changes: 1 addition & 1 deletion styleguide/source/_patterns/01-atoms/04-headings/comp-heading~yellow.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"compHeading": {
"title": "Title text",
"title": "Comp Heading",
"sub": false,
"color": "yellow",
"id": "",
Expand Down
30 changes: 22 additions & 8 deletions styleguide/source/_patterns/03-organisms/by-author/action-finder.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,17 @@
"id": "GUID831741781374893",
"bgWide":"../../assets/images/placeholder/1600x600-lighthouse-blur.jpg",
"bgNarrow":"../../assets/images/placeholder/800x800.png",
"title": "What Would You Like to Do?",
"featuredHeading":"Featured:",
"generalHeading":"All Actions & Guides:",

"compHeading": {
"title": "What Would You Like to Do?",
"sub": false,
"color": "yellow",
"id": "",
"centered": false
},

"seeAll": null,

"featuredLinks": [{
Expand All @@ -19,24 +26,28 @@
"image": "",
"text": "Cancel or Transfer a Campsite Reservation",
"type": "external",
"href": "#"
"href": "#",
"label": ""
},{
"image": "",
"text": "Find Horseback Riding Trails",
"type": "",
"href": "#"
"href": "#",
"label": ""
}],

"links": [{
"image": "",
"text": "Find a State Park",
"type": "",
"href": "#"
"href": "#",
"label": ""
},{
"image": "",
"text": "Get a State Park Pass",
"type": "",
"href": "#"
"href": "#",
"label": ""
},{
"image": "../../assets/images/placeholder/130x160.png",
"text": "Fall Activity Planner",
Expand All @@ -47,17 +58,20 @@
"image": "",
"text": "Reserve a Campsite",
"type": "external",
"href": "#"
"href": "#",
"label": ""
},{
"image": "",
"text": "Find Scenic Viewing Areas",
"type": "",
"href": "#"
"href": "#",
"label": ""
},{
"image": "",
"text": "Download a Trail Map",
"type": "",
"href": "#"
"href": "#",
"label": ""
}]
}
}
8 changes: 4 additions & 4 deletions styleguide/source/_patterns/03-organisms/by-author/action-finder.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ A collection of featured and general links
* Stable as of 5.0.0

### Pattern Contains
* Callout Link
* Illustrated Link
* Decorative Link
* Comp Heading
* Key Actions

### Variant options
* [Without a Background](./?p=organisms-action-finder-without-background)
Expand All @@ -24,12 +24,12 @@ actionFinder: {
type: string (image path) / optional / (required with bgNarrow)
bgNarrow:
type: string (image path) / optional
title:
type: string / required
featuredHeading:
type: string / required
generalHeading:
type: string / required
compHeading:
type: compHeading / required

seeAll: {
type: decorativeLink / optional
Expand Down
48 changes: 19 additions & 29 deletions styleguide/source/_patterns/03-organisms/by-author/action-finder.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,40 +20,30 @@
{% endif %}
<div class="ma__action-finder__container">
<header class="ma__action-finder__header">
<h2 class="ma__action-finder__title">
{{ actionFinder.title }}
</h2>
{# Backward compatible with 5.7 - replaced h2 with compHeading #}
Copy link
Copy Markdown
Contributor

@jesconstantine jesconstantine Sep 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

{% set compHeading = actionFinder.compHeading ? : {"title": actionFinder.title, "color":"yellow","id": ""} %}
Copy link
Copy Markdown
Contributor

@jesconstantine jesconstantine Sep 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we want the title heading to have an id since that could potentially be passed down to the keyActions aria-labelledby (i.e. in the else clause for the general items)

Copy link
Copy Markdown
Author

@gender404 gender404 Sep 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The aria-labelledby id would be the sub heading not the this title.

{% include "@atoms/04-headings/comp-heading.twig" %}
</header>
{% if actionFinder.featuredLinks %}
{% if actionFinder.featuredHeading %}
<h3 class="ma__action-finder__category">{{ actionFinder.featuredHeading }}</h3>
{% endif %}
<div class="ma__action-finder__items">
Copy link
Copy Markdown
Contributor

@jesconstantine jesconstantine Sep 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw the backwards-compatible scss to support this selector - thank you!

{% for link in actionFinder.featuredLinks %}
{% if link.image %}
{% set illustratedLink = link %}
{% include "@molecules/illustrated-link.twig" %}
{% else %}
{% set calloutLink = link %}
{% include "@molecules/callout-link.twig" %}
{% endif %}
{% endfor %}
<div class="ma__action-finder__featured-items">
{% set keyActions = { "links": actionFinder.featuredLinks } %}
Copy link
Copy Markdown
Contributor

@jesconstantine jesconstantine Sep 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures that the data structure can stay the same - nice!

{% if actionFinder.featuredHeading %}
<h3 class="ma__action-finder__category" id="featured-{{ actionFinder.id }}">{{ actionFinder.featuredHeading }}</h3>
{% set keyActions = keyActions|merge({"id": "featured-" ~ actionFinder.id, }) %}
{% endif %}
{% include "@organisms/by-author/key-actions.twig" %}
Copy link
Copy Markdown
Contributor

@jesconstantine jesconstantine Sep 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the id for keyActions if we don't have a featuredHeading here?

Copy link
Copy Markdown
Author

@gender404 gender404 Sep 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, now I get the other comment. I'll pass the main title's id if we don't have a featured Heading.

</div>
{% endif %}
{% if actionFinder.links %}
{% if actionFinder.generalHeading and actionFinder.featuredLinks %}
<h3 class="ma__action-finder__category">{{ actionFinder.generalHeading }}</h3>
{% endif %}
<div class="ma__action-finder__items ma__action-finder__items--all">
Copy link
Copy Markdown
Contributor

@jesconstantine jesconstantine Sep 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw the backwards-compatible scss to support this selector, thank you!

{% for link in actionFinder.links %}
{% if link.image %}
{% set illustratedLink = link %}
{% include "@molecules/illustrated-link.twig" %}
{% else %}
{% set calloutLink = link %}
{% include "@molecules/callout-link.twig" %}
{% endif %}
{% endfor %}
<div class="ma__action-finder__all-items">
{% set keyActions = { "links": actionFinder.links } %}
Copy link
Copy Markdown
Contributor

@jesconstantine jesconstantine Sep 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures that the data structure can stay the same - nice!

{% if actionFinder.generalHeading and actionFinder.featuredLinks %}
Copy link
Copy Markdown
Contributor

@jesconstantine jesconstantine Sep 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why wouldn't we display the generalHeading regardless of whether or not there were featuredLinks?

Copy link
Copy Markdown
Author

@gender404 gender404 Sep 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was a design decision. Did really make sense to say "All ..." when there aren't "featured" links above it. The heading just adds confusion.

<h3 class="ma__action-finder__category" id="all-{{ actionFinder.id }}">{{ actionFinder.generalHeading }}</h3>
{% set keyActions = keyActions|merge({"id": "all-" ~ actionFinder.id, }) %}
{% else %}
{% set keyActions = keyActions|merge({"id": actionFinder.id, }) %}
Copy link
Copy Markdown
Contributor

@jesconstantine jesconstantine Sep 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actionFinder.id doesn't currently correspond to a heading, does it?

Copy link
Copy Markdown
Author

@gender404 gender404 Sep 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

{% endif %}
{% include "@organisms/by-author/key-actions.twig" %}
</div>
{% endif %}
</div>
Expand Down
5 changes: 5 additions & 0 deletions ...uide/source/_patterns/03-organisms/by-author/key-actions-with-hidden-heading.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
### Description
This is a variant of the [Key Actions](./?p=organisms-key-actions) pattern showing an example with the heading visually hidden to provide context for screen readers.

### How to generate
* set the `hiddenHeading` variable to true
20 changes: 15 additions & 5 deletions styleguide/source/_patterns/03-organisms/by-author/key-actions.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,42 @@
{
"keyActions": {
"id": "GUID0828340",
"hiddenHeading": false,
"compHeading": {
"title": "Key Actions",
"sub": false,
"id": "key-action-id"
"id": ""
},
"links":[{
"image": "",
"text": "Register to Vote",
"type": "",
"href": "#"
"href": "#",
"label": ""
},{
"image": "",
"text": "Find a Job",
"type": "",
"href": "#"
"href": "#",
"label": ""
},{
"image": "../../assets/images/placeholder/130x160.png",
"text": "File for unemployment",
"type": "",
"href": "#",
"label": "Guide:"
},{
"image": "",
"text": "Learn more about the tax code",
"type": "",
"href": "#"
"href": "#",
"label": ""
},{
"image": "",
"text": "Learn more about the tax code",
"type": "",
"href": "#"
"href": "#",
"label": ""
}]
}
}
18 changes: 9 additions & 9 deletions styleguide/source/_patterns/03-organisms/by-author/key-actions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,19 @@ This pattern shows a grid containing a combination of Callout links and Illustra
* Illustrated Link molecule (optional)

### Variant options
* This pattern can also contain Illustrated Links in addition to just Callout Links.

* Can be rendered with the title [visually hidden](./?p=organisms-key-actions-with-hidden-heading) to provide context for screen readers.

### Variables
~~~
"keyActions": {
"compHeading": {
"type": compHeading / optional
},
sidebarHeading: {
type: sidebarHeading / optional
keyActions: {
id:
type: string (unique per page) / optional
hiddenHeading:
type: boolean
compHeading: {
type: compHeading / optional
},
"links":[{
links:[{
type: array of Callout Links or Illustrated Links / required
}]
}
Expand Down
19 changes: 16 additions & 3 deletions styleguide/source/_patterns/03-organisms/by-author/key-actions.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,26 @@
{% set keyActions = keyActions|merge({"compHeading": { "title": keyActions.sidebarHeading.title, "sidebar" : true }}) %}
{% endif %}

<section class="ma__key-actions">
{# fallback to compHeading Id value #}
{% set keyActionId = keyActions.compHeading.id %}
Copy link
Copy Markdown
Contributor

@jesconstantine jesconstantine Sep 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check for keyActions.compHeading.id here first since it's optional?


{% if keyActions.id %}
{% set keyActionId = keyActions.id %}
{% endif %}

{% set hiddenClass = keyActions.hiddenHeading ? 'ma__key-actions--hidden-heading' : '' %}

<section class="ma__key-actions {{ hiddenClass }}">
<div class="ma__key-actions__container">
{% if keyActions.compHeading %}
{% set compHeading = keyActions.compHeading %}
{% set compHeading = keyActions.compHeading|merge({"id": keyActionId}) %}
Copy link
Copy Markdown
Contributor

@jesconstantine jesconstantine Sep 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why this is necessary? (I know we talked about it!) If there is a compHeading.id don't we always want to use that? And if there isn't a compHeading.id isn't it likely that the id was passed in because there is a heading in the parent component that is including this whose id is used in aria-labelledby?

Copy link
Copy Markdown
Author

@gender404 gender404 Sep 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we want to always use is the keyActions.id value. If that hasn't been passed then we fallback to the compHeading.id. If that hasn't been passed then we fall back to a blank id, because unlabeled is better than mis-labeled.

{% include "@atoms/04-headings/comp-heading.twig" %}
{% endif %}
<div class="ma__key-actions__items" aria-labelledby="{{ keyActions.compHeading.id }}">
<div
class="ma__key-actions__items"
{% if keyActionId %}
aria-labelledby="{{ keyActionId }}"
Copy link
Copy Markdown
Contributor

@jesconstantine jesconstantine Sep 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember talking about the heading / labelledby id stuff, just want to make sure I understand what's happening here:

When key actions is used as a standalone component, it will generally have a compHeading (visual or semantic) with an id, so that is the default / fallback value of keyActionId -- a variable which refers to the id of the thing that describes it.

When key actions is included as a child component (i.e. actionFinder), it does not need to use its compHeading because the parent component could have its own heading and can therefor pass in an id for that heading as keyActions.id (i.e. keyActions|merge({"id": "all-" ~ actionFinder.id, }). This variable assignment happens after the fallback so it will always take precedent if it exists.

Am I understanding this correctly?

Copy link
Copy Markdown
Author

@gender404 gender404 Sep 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct.

{% endif %}>
{% for link in keyActions.links %}
{% if link.image %}
{% set illustratedLink = link %}
Expand Down
34 changes: 34 additions & 0 deletions styleguide/source/_patterns/03-organisms/by-author/key-actions~with-hidden-heading.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{
"keyActions": {
"id": "GUID0828340",
"hiddenHeading": true,
"compHeading": {
"title": "Key Actions",
"sub": false,
"id": ""
},
"links":[{
"text": "Register to Vote",
"type": "",
"href": "#"
},{
"text": "Find a Job",
"type": "",
"href": "#"
},{
"image": "../../assets/images/placeholder/130x160.png",
"text": "File for unemployment",
"type": "",
"href": "#",
"label": "Guide:"
},{
"text": "Learn more about the tax code",
"type": "",
"href": "#"
},{
"text": "Learn more about the tax code",
"type": "",
"href": "#"
}]
}
}
Loading