From c4ae1a7ea98ccb945966cd15897c7cd2ebbe26e3 Mon Sep 17 00:00:00 2001 From: Richard McCarthy Date: Mon, 28 Oct 2024 08:51:38 +0000 Subject: [PATCH 1/9] refactor summary component to remove extra div in dl --- src/components/summary/_macro.njk | 149 +++++++++++++-------------- src/components/summary/_summary.scss | 25 +++-- 2 files changed, 89 insertions(+), 85 deletions(-) diff --git a/src/components/summary/_macro.njk b/src/components/summary/_macro.njk index e4f4a1617a..d8407145d8 100644 --- a/src/components/summary/_macro.njk +++ b/src/components/summary/_macro.njk @@ -33,7 +33,9 @@ {% set itemClass = "" %} {% if row.error %}{% set itemClass = " ons-summary__item--error" %}{% endif %} {% if row.total %}{% set itemClass = itemClass + " ons-summary__item--total" %}{% endif %} -
+
{% if row.errorMessage %}
{{ row.errorMessage }}
{% endif %} @@ -44,87 +46,82 @@ {% endif %} {% for item in row.itemsList %} -
-
- {% if item.iconType %} - {% from "components/icon/_macro.njk" import onsIcon %} - - {{- - onsIcon({ - "iconType": item.iconType - }) - -}} - {% if item.iconVisuallyHiddenText %} - {{ item.iconVisuallyHiddenText }} - {% endif %} - - {% endif %} - -
- {{- item.title | default(row.title) | safe -}} -
+ {{- + onsIcon({ + "iconType": item.iconType + }) + -}} + {% if item.iconVisuallyHiddenText %} + {{ item.iconVisuallyHiddenText }} + {% endif %} + + {% endif %} - {# Render section status for mobile if is hub #} - {% if variantHub and item.valueList %} - — {{ item.valueList[0].text | safe }} - {% endif %} -
- {% if item.valueList %} -
- {% if item.valueList | length == 1 %} - {{ item.valueList[0].text | safe }} - {% if item.valueList[0].other or item.valueList[0].other == 0 %} -
    -
  • {{ item.valueList[0].other | safe }}
    • -
- {% endif %} - {% else %} +
+ {{- item.title | default(row.title) | safe -}} +
+ + {# Render section status for mobile if is hub #} + {% if variantHub and item.valueList %} + — {{ item.valueList[0].text | safe }} + {% endif %} + + {% if item.valueList %} +
+ {% if item.valueList | length == 1 %} + {{ item.valueList[0].text | safe }} + {% if item.valueList[0].other or item.valueList[0].other == 0 %}
    - {% for value in item.valueList %} -
  • - {{ value.text | safe }} - {% if value.other or value.other == 0 %} -
      -
    • {{ value.other | safe }}
      • -
    - {% endif %} -
    • - {% endfor %} +
  • {{ item.valueList[0].other | safe }}
{% endif %} -
- {% endif %} - {% if item.actions %} -
- {% for action in item.actions %} - {% if loop.index > 1 %}{% endif %} - - - {{ action.visuallyHiddenText }} - - {% endfor %} -
- {% endif %} -
+ {% else %} +
    + {% for value in item.valueList %} +
  • + {{ value.text | safe }} + {% if value.other or value.other == 0 %} +
      +
    • {{ value.other | safe }}
      • +
    + {% endif %} +
    • + {% endfor %} +
+ {% endif %} + + {% endif %} + {% if item.actions %} +
+ {% for action in item.actions %} + {% if loop.index > 1 %}{% endif %} + + + {{ action.visuallyHiddenText }} + + {% endfor %} +
+ {% endif %} {% endfor %}
{% endfor %} diff --git a/src/components/summary/_summary.scss b/src/components/summary/_summary.scss index 6c634b7354..5e2c7984f5 100644 --- a/src/components/summary/_summary.scss +++ b/src/components/summary/_summary.scss @@ -18,17 +18,17 @@ $hub-row-spacing: 1.3rem; } } - &__row { + &__item { display: flex; margin: 0; - } - - &__item { + flex-wrap: wrap; &:not(:last-child), &:not(.ons-summary__group--card .ons-summary__item):nth-of-type(1) { border-bottom: 1px solid var(--ons-color-borders); } + } + &__item { &--total { @extend .ons-u-fs-m; @@ -51,6 +51,7 @@ $hub-row-spacing: 1.3rem; &__row-title { padding: $summary-row-spacing 0; text-align: left; + flex: 0 0 100%; } // reduces the gap between row title and summary title when there is no group title &__title + &__group &__row-title--no-group-title { @@ -95,12 +96,13 @@ $hub-row-spacing: 1.3rem; width: 1px; } - // Item Modifiers + // Row Modifiers &__item--error & { &__row-title--error { color: var(--ons-color-errors); font-weight: $font-weight-bold; padding: $summary-row-spacing $summary-col-spacing; + flex: 0 0 100%; } &__row-title, @@ -159,7 +161,7 @@ $hub-row-spacing: 1.3rem; } } - &__row { + &__item { flex-direction: column; } } @@ -172,14 +174,19 @@ $hub-row-spacing: 1.3rem; flex: 5 1 33%; padding-top: $summary-row-spacing; vertical-align: top; - &:not(:last-child) { - padding-right: $summary-col-spacing; - } + } + + &__item-title, + &__values { + padding-right: $summary-col-spacing; } &__actions { display: flex; justify-content: right; + & :not(.ons-summary__item-title--error) { + padding-right: 0; + } } &__button { From 5c4d79dec7089f4a868981f205f29428346c8e80 Mon Sep 17 00:00:00 2001 From: Richard McCarthy Date: Mon, 28 Oct 2024 09:29:42 +0000 Subject: [PATCH 2/9] remove has-values class no longer used --- src/components/summary/_macro.njk | 4 +--- src/components/summary/_macro.spec.js | 6 ------ 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/components/summary/_macro.njk b/src/components/summary/_macro.njk index d8407145d8..8d73759da5 100644 --- a/src/components/summary/_macro.njk +++ b/src/components/summary/_macro.njk @@ -33,9 +33,7 @@ {% set itemClass = "" %} {% if row.error %}{% set itemClass = " ons-summary__item--error" %}{% endif %} {% if row.total %}{% set itemClass = itemClass + " ons-summary__item--total" %}{% endif %} -
+
{% if row.errorMessage %}
{{ row.errorMessage }}
{% endif %} diff --git a/src/components/summary/_macro.spec.js b/src/components/summary/_macro.spec.js index 3240e26e89..102f249245 100644 --- a/src/components/summary/_macro.spec.js +++ b/src/components/summary/_macro.spec.js @@ -399,12 +399,6 @@ describe('macro: summary', () => { expect($('#row-id-3').length).toBe(1); }); - it('has the correct class for each row when there is a `valueList`', () => { - const $ = cheerio.load(renderComponent('summary', EXAMPLE_SUMMARY_BASIC)); - - expect($('.ons-summary__row--has-values').length).toBe(5); - }); - it('has custom row `titleAttributes`', () => { const $ = cheerio.load(renderComponent('summary', EXAMPLE_SUMMARY_BASIC)); From 65f50605a0b43c57e577d01e62b4552a26a704a5 Mon Sep 17 00:00:00 2001 From: Richard McCarthy Date: Mon, 28 Oct 2024 09:44:17 +0000 Subject: [PATCH 3/9] fix tests --- src/components/summary/_macro.spec.js | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/components/summary/_macro.spec.js b/src/components/summary/_macro.spec.js index 102f249245..0ca51ee53d 100644 --- a/src/components/summary/_macro.spec.js +++ b/src/components/summary/_macro.spec.js @@ -456,18 +456,16 @@ describe('macro: summary', () => { const $ = cheerio.load(renderComponent('summary', EXAMPLE_SUMMARY_BASIC)); expect( - $('.ons-summary__items .ons-summary__item:nth-of-type(1) .ons-summary__row .ons-summary__values .ons-summary__text') - .text() - .trim(), + $('.ons-summary__items .ons-summary__item .ons-summary__values:nth-of-type(1) .ons-summary__text').text().trim(), ).toBe('row value 1'); }); it('displays the `other` text', () => { const $ = cheerio.load(renderComponent('summary', EXAMPLE_SUMMARY_BASIC)); - expect( - $('.ons-summary__items .ons-summary__item:nth-of-type(1) .ons-summary__row .ons-summary__values ul li').text().trim(), - ).toBe('other value'); + expect($('.ons-summary__items .ons-summary__item .ons-summary__values:nth-of-type(1) ul li').text().trim()).toBe( + 'other value', + ); }); it('wraps the `valueList` in a ul if multiple values provided', () => { @@ -595,9 +593,7 @@ describe('macro: summary', () => { }), ); - expect($('.ons-summary__items .ons-summary__item:nth-of-type(2) .ons-summary__row .ons-summary__item-title span').text()).toBe( - ' — row value 2', - ); + expect($('.ons-summary__items .ons-summary__item .ons-summary__item-title:nth-of-type(2) span').text()).toBe(' — row value 2'); }); }); From ccf15894d56d63add88e7f80b60bee3e2d9b25c4 Mon Sep 17 00:00:00 2001 From: Richard McCarthy Date: Mon, 28 Oct 2024 10:14:17 +0000 Subject: [PATCH 4/9] fix tests --- src/components/summary/_macro.spec.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/components/summary/_macro.spec.js b/src/components/summary/_macro.spec.js index 0ca51ee53d..7bd21bf159 100644 --- a/src/components/summary/_macro.spec.js +++ b/src/components/summary/_macro.spec.js @@ -456,16 +456,18 @@ describe('macro: summary', () => { const $ = cheerio.load(renderComponent('summary', EXAMPLE_SUMMARY_BASIC)); expect( - $('.ons-summary__items .ons-summary__item .ons-summary__values:nth-of-type(1) .ons-summary__text').text().trim(), + $('.ons-summary__items .ons-summary__item .ons-summary__items:nth-of-type(1) .ons-summary__values .ons-summary__text') + .text() + .trim(), ).toBe('row value 1'); }); it('displays the `other` text', () => { const $ = cheerio.load(renderComponent('summary', EXAMPLE_SUMMARY_BASIC)); - expect($('.ons-summary__items .ons-summary__item .ons-summary__values:nth-of-type(1) ul li').text().trim()).toBe( - 'other value', - ); + expect( + $('.ons-summary__items .ons-summary__item .ons-summary__items:nth-of-type(1) .ons-summary__values ul li').text().trim(), + ).toBe('other value'); }); it('wraps the `valueList` in a ul if multiple values provided', () => { @@ -593,7 +595,7 @@ describe('macro: summary', () => { }), ); - expect($('.ons-summary__items .ons-summary__item .ons-summary__item-title:nth-of-type(2) span').text()).toBe(' — row value 2'); + expect($('.ons-summary__item .ons-summary__items:nth-of-type(2) .ons-summary__item-title span').text()).toBe(' — row value 2'); }); }); From 0b3bc2001aa3ca65d45ce359210cd0d9dc6d62b9 Mon Sep 17 00:00:00 2001 From: Richard McCarthy Date: Mon, 28 Oct 2024 10:36:13 +0000 Subject: [PATCH 5/9] fix tests --- src/components/summary/_macro.spec.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/components/summary/_macro.spec.js b/src/components/summary/_macro.spec.js index 7bd21bf159..80843f6579 100644 --- a/src/components/summary/_macro.spec.js +++ b/src/components/summary/_macro.spec.js @@ -456,18 +456,16 @@ describe('macro: summary', () => { const $ = cheerio.load(renderComponent('summary', EXAMPLE_SUMMARY_BASIC)); expect( - $('.ons-summary__items .ons-summary__item .ons-summary__items:nth-of-type(1) .ons-summary__values .ons-summary__text') - .text() - .trim(), + $('.ons-summary__items .ons-summary__item:nth-of-type(1) .ons-summary__values .ons-summary__text').text().trim(), ).toBe('row value 1'); }); it('displays the `other` text', () => { const $ = cheerio.load(renderComponent('summary', EXAMPLE_SUMMARY_BASIC)); - expect( - $('.ons-summary__items .ons-summary__item .ons-summary__items:nth-of-type(1) .ons-summary__values ul li').text().trim(), - ).toBe('other value'); + expect($('.ons-summary__items .ons-summary__item:nth-of-type(1) .ons-summary__values ul li').text().trim()).toBe( + 'other value', + ); }); it('wraps the `valueList` in a ul if multiple values provided', () => { @@ -595,7 +593,7 @@ describe('macro: summary', () => { }), ); - expect($('.ons-summary__item .ons-summary__items:nth-of-type(2) .ons-summary__item-title span').text()).toBe(' — row value 2'); + expect($('.ons-summary__items .ons-summary__item:nth-of-type(2) .ons-summary__item-title span').text()).toBe(' — row value 2'); }); }); From 86f7a6e16e1da97b5f01e627bf7d2953c6ca1f98 Mon Sep 17 00:00:00 2001 From: Richard McCarthy Date: Mon, 28 Oct 2024 10:47:34 +0000 Subject: [PATCH 6/9] mark summary titles as dts --- src/components/summary/_macro.njk | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/summary/_macro.njk b/src/components/summary/_macro.njk index 8d73759da5..352b956ad8 100644 --- a/src/components/summary/_macro.njk +++ b/src/components/summary/_macro.njk @@ -35,12 +35,12 @@ {% if row.total %}{% set itemClass = itemClass + " ons-summary__item--total" %}{% endif %}
{% if row.errorMessage %} -
{{ row.errorMessage }}
+
{{ row.errorMessage }}
{% endif %} {% if row.itemsList | length > 1 and row.title %} -
+
{{- row.title -}} -
+ {% endif %} {% for item in row.itemsList %} From 7a99b42c86fb27b22f383d405c63217a33216186 Mon Sep 17 00:00:00 2001 From: Richard McCarthy Date: Mon, 28 Oct 2024 11:19:52 +0000 Subject: [PATCH 7/9] tidy up css --- src/components/summary/_summary.scss | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/components/summary/_summary.scss b/src/components/summary/_summary.scss index 5e2c7984f5..b0c784d2cd 100644 --- a/src/components/summary/_summary.scss +++ b/src/components/summary/_summary.scss @@ -26,9 +26,7 @@ $hub-row-spacing: 1.3rem; &:not(.ons-summary__group--card .ons-summary__item):nth-of-type(1) { border-bottom: 1px solid var(--ons-color-borders); } - } - &__item { &--total { @extend .ons-u-fs-m; From 5e2d10c1ed2110e3501d2e37930d90bb7ff08355 Mon Sep 17 00:00:00 2001 From: Richard McCarthy Date: Mon, 28 Oct 2024 11:51:38 +0000 Subject: [PATCH 8/9] add class to container element --- src/components/summary/_macro.njk | 2 +- src/components/summary/_summary.scss | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/summary/_macro.njk b/src/components/summary/_macro.njk index 352b956ad8..33ba88d346 100644 --- a/src/components/summary/_macro.njk +++ b/src/components/summary/_macro.njk @@ -141,7 +141,7 @@ {% endset %} {% if params.variant == 'card' %} -
{{ link | safe }}
+
{{ link | safe }}
{% else %} {{ link | safe }} {% endif %} diff --git a/src/components/summary/_summary.scss b/src/components/summary/_summary.scss index b0c784d2cd..38a7323d39 100644 --- a/src/components/summary/_summary.scss +++ b/src/components/summary/_summary.scss @@ -220,7 +220,7 @@ $hub-row-spacing: 1.3rem; } .ons-summary__link { padding: 0 1.25rem; - div { + &-container { padding: 1rem 0; border-top: 1px solid var(--ons-color-borders-light); } @@ -230,7 +230,7 @@ $hub-row-spacing: 1.3rem; padding: 0.5rem 1.25rem 0; } .ons-summary__placeholder + .ons-summary__link { - div { + &-container { border-top: 0; } } From 7dee403dcd5ec734abb9e195c46130dfbee3dafa Mon Sep 17 00:00:00 2001 From: Richard McCarthy Date: Fri, 1 Nov 2024 14:02:48 +0000 Subject: [PATCH 9/9] remove id from params list --- src/components/summary/_macro-options.md | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/summary/_macro-options.md b/src/components/summary/_macro-options.md index 31ec6ecb60..52c01010b1 100644 --- a/src/components/summary/_macro-options.md +++ b/src/components/summary/_macro-options.md @@ -36,7 +36,6 @@ | Name | Type | Required | Description | | ---------------------- | ---------------------- | -------- | ------------------------------------------------------------------------------------------- | -| id | string | false | The HTML `id` of the row item | | iconType | string | false | Adds an icon before the row title, by setting the [icon type](/foundations/icons#icon-type) | | iconVisuallyHiddenText | string | false | Visually hidden text in a span under the icon to add more context for screen readers | | title | string | false | The title for the row item |