From cd2171720d4b9c90ad7f61090799303059f5bd57 Mon Sep 17 00:00:00 2001 From: Tyler Sticka Date: Tue, 7 Dec 2021 13:01:25 -0800 Subject: [PATCH 1/4] Fix for Safari list column issues --- src/objects/list/list.scss | 51 ++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/src/objects/list/list.scss b/src/objects/list/list.scss index 787fb8714..5f1f0ab94 100644 --- a/src/objects/list/list.scss +++ b/src/objects/list/list.scss @@ -4,33 +4,29 @@ @use '../../mixins/fluid'; @use '../../mixins/media-query'; -/** - * Override browser defaults. Note that this means it's very important to - * include `[role="list"]` to prevent disruption of list navigation in Safari - * VoiceOver. - * - * @see https://www.scottohara.me/blog/2019/01/12/lists-and-safari.html - */ +/// Override browser defaults. Note that this means it's very important to +/// include `[role="list"]` to prevent disruption of list navigation in Safari +/// VoiceOver, unless the content itself makes its nature obvious. +/// +/// @link https://www.scottohara.me/blog/2019/01/12/lists-and-safari.html .o-list { list-style: none; padding-inline-start: 0; } -/** - * Modifier: Inline - * - * By default, this uses negative margins on the sides to account for extra - * margin between child elements. This allows child elements to reach the edges - * of their parent while maintaining consistent gaps between. - * - * This would be a _lot_ more elegant if it used the Flexbox-compatible `gap` - * property. But unfortunately there is no way to test for that without false - * positives as of this writing. - * - * @see https://medium.com/@schofeld/mind-the-flex-gap-c9cd1b4b35d8 - * @see https://github.com/w3c/csswg-drafts/issues/3559 - */ +/// Modifier: Inline +/// +/// By default, this uses negative margins on the sides to account for extra +/// margin between child elements. This allows child elements to reach the edges +/// of their parent while maintaining consistent gaps between. +/// +/// This would be a _lot_ more elegant if it used the Flexbox-compatible `gap` +/// property. But unfortunately there is no way to test for that without false +/// positives as of this writing. +/// +/// @link https://medium.com/@schofeld/mind-the-flex-gap-c9cd1b4b35d8 +/// @link https://github.com/w3c/csswg-drafts/issues/3559 .o-list--inline { display: flex; @@ -52,6 +48,19 @@ breakpoint.$xl ); columns: #{$i}; + position: relative; + + // Safari has some odd bugs related to CSS multi-column. Specifically: + // + // 1. Without this, the top of an item may appear in one column with the + // bottom half in the following column. + // 2. Without this, `position: absolute` elements will impact the overall + // window scroll height, even if they're invisible (as with our + // `u-hidden-visually` utility). + > * { + break-inside: avoid; // 1 + position: relative; // 2 + } } } } From 7437715678c16ea8229492ae62a91794db6388d5 Mon Sep 17 00:00:00 2001 From: Tyler Sticka Date: Tue, 7 Dec 2021 13:07:48 -0800 Subject: [PATCH 2/4] Use padding instead of space for list examples --- src/objects/overview/demo/advanced.twig | 6 +++++- src/prototypes/article-listing/example/example.twig | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/objects/overview/demo/advanced.twig b/src/objects/overview/demo/advanced.twig index 220d04e75..22ab3bc63 100644 --- a/src/objects/overview/demo/advanced.twig +++ b/src/objects/overview/demo/advanced.twig @@ -26,7 +26,11 @@ } %} {% block content %} {% for topic in topics %} -
  • + {# + We must use padding between items because Safari likes to carry + margin across columns. + #} +
  • {% include '@cloudfour/components/dot-leader/dot-leader.twig' with { label: topic.title, count: topic.count, diff --git a/src/prototypes/article-listing/example/example.twig b/src/prototypes/article-listing/example/example.twig index 3764275f5..e44930491 100644 --- a/src/prototypes/article-listing/example/example.twig +++ b/src/prototypes/article-listing/example/example.twig @@ -203,7 +203,11 @@ } %} {% block content %} {% for topic in topics %} -
  • + {# + We must use padding between items because Safari likes to + carry margin across columns. + #} +
  • {% include '@cloudfour/components/dot-leader/dot-leader.twig' with { label: topic.title, count: topic.count, From 0fbd16bec28ab7fee93fa9d7bb52aa53e78acc2f Mon Sep 17 00:00:00 2001 From: Tyler Sticka Date: Tue, 7 Dec 2021 13:08:53 -0800 Subject: [PATCH 3/4] Add changeset --- .changeset/ten-candles-guess.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/ten-candles-guess.md diff --git a/.changeset/ten-candles-guess.md b/.changeset/ten-candles-guess.md new file mode 100644 index 000000000..09281b979 --- /dev/null +++ b/.changeset/ten-candles-guess.md @@ -0,0 +1,5 @@ +--- +'@cloudfour/patterns': patch +--- + +Fix for List objects with columns in Safari that include visually hidden elements or elements with content divided between columns From 98dbd9aabffd0312e3204c2da33c5b6dd859429a Mon Sep 17 00:00:00 2001 From: Tyler Sticka Date: Tue, 7 Dec 2021 13:11:40 -0800 Subject: [PATCH 4/4] Lint fix --- src/objects/list/list.scss | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/objects/list/list.scss b/src/objects/list/list.scss index 5f1f0ab94..48726464a 100644 --- a/src/objects/list/list.scss +++ b/src/objects/list/list.scss @@ -50,13 +50,13 @@ columns: #{$i}; position: relative; - // Safari has some odd bugs related to CSS multi-column. Specifically: - // - // 1. Without this, the top of an item may appear in one column with the - // bottom half in the following column. - // 2. Without this, `position: absolute` elements will impact the overall - // window scroll height, even if they're invisible (as with our - // `u-hidden-visually` utility). + /// Safari has some odd bugs related to CSS multi-column. Specifically: + /// + /// 1. Without this, the top of an item may appear in one column with the + /// bottom half in the following column. + /// 2. Without this, `position: absolute` elements will impact the overall + /// window scroll height, even if they're invisible (as with our + /// `u-hidden-visually` utility). > * { break-inside: avoid; // 1 position: relative; // 2