fix: review

sbb-design-systems · May 21, 2024 · 844fc8a · 844fc8a
1 parent 298f7c8
commit 844fc8a
Show file tree

Hide file tree

Showing 7 changed files with 102 additions and 122 deletions.
diff --git a/src/components/core/testing/private.ts b/src/components/core/testing/private.ts
@@ -4,6 +4,7 @@ export * from './private/describe-viewports.js';
 export * from './private/dispatch-events.js';
 export * from './private/event-objects.js';
 export * from './private/fixture.js';
+export * from './private/load-asset-as-base64.js';
 export * from './private/platform.js';
 export * from './private/type-in-element.js';
 export * from './private/visual-regression-snapshot.js';
diff --git a/src/components/core/testing/private/load-asset-as-base64.ts b/src/components/core/testing/private/load-asset-as-base64.ts
@@ -0,0 +1,3 @@
+export async function loadAssetAsBase64(url: string): Promise<string> {
+  return URL.createObjectURL(await fetch(url).then((r) => r.blob()));
+}
diff --git a/src/components/lead-container/__snapshots__/lead-container.snapshot.spec.snap.js b/src/components/lead-container/__snapshots__/lead-container.snapshot.spec.snap.js
@@ -1,15 +1,19 @@
 /* @web/test-runner snapshot v1 */
 export const snapshots = {};
 
-snapshots["sbb-lead-container DOM"] = 
+snapshots["sbb-lead-container DOM"] =
 `<sbb-lead-container>
-  <sbb-image slot="image">
+  <sbb-image
+    aspect-ratio="16-9"
+    border-radius="default"
+    slot="image"
+  >
   </sbb-image>
 </sbb-lead-container>
 `;
 /* end snapshot sbb-lead-container DOM */
 
-snapshots["sbb-lead-container Shadow DOM"] = 
+snapshots["sbb-lead-container Shadow DOM"] =
 `<div class="sbb-lead-container">
   <div class="sbb-lead-container-image">
     <slot name="image">
@@ -25,7 +29,7 @@ snapshots["sbb-lead-container Shadow DOM"] =
 `;
 /* end snapshot sbb-lead-container Shadow DOM */
 
-snapshots["sbb-lead-container A11y tree Chrome"] = 
+snapshots["sbb-lead-container A11y tree Chrome"] =
 `<p>
   {
   "role": "WebArea",
@@ -35,7 +39,7 @@ snapshots["sbb-lead-container A11y tree Chrome"] =
 `;
 /* end snapshot sbb-lead-container A11y tree Chrome */
 
-snapshots["sbb-lead-container A11y tree Firefox"] = 
+snapshots["sbb-lead-container A11y tree Firefox"] =
 `<p>
   {
   "role": "document",

diff --git a/src/components/lead-container/lead-container.scss b/src/components/lead-container/lead-container.scss
@@ -7,7 +7,7 @@
   display: block;
 
   --sbb-lead-container-background-color: var(--sbb-color-white);
-  --sbb-lead-container-image-ratio: 2/1;
+  --sbb-lead-container-image-ratio: 2 / 1;
 
   // TODO: Check if token needed
   --sbb-lead-container-image-overlap: -6rem;
@@ -16,35 +16,44 @@
 
   // TODO: Check if new border-radius token needed
   --sbb-lead-container-border-radius: var(--sbb-spacing-fixed-3x);
+  --sbb-lead-container-image-border-radius: 0;
 
   @include sbb.mq($from: wide) {
-    --sbb-lead-container-image-ratio: 21/9;
+    --sbb-lead-container-image-ratio: 21 / 9;
+  }
+
+  @include sbb.mq($from: ultra) {
+    --sbb-lead-container-image-border-radius: var(--sbb-lead-container-border-radius);
   }
 }
 
 .sbb-lead-container {
   padding-block-end: var(--sbb-spacing-responsive-l);
 }
 
-// TODO: Check if default aspect ratio should be `free` of sbb-image
 ::slotted(sbb-image[slot='image']) {
   --sbb-image-aspect-ratio: var(--sbb-lead-container-image-ratio);
-  --sbb-image-border-radius: 0;
-
-  @include sbb.mq($from: ultra) {
-    --sbb-image-border-radius: var(--sbb-lead-container-border-radius);
-  }
+  --sbb-image-border-radius: var(--sbb-lead-container-image-border-radius);
 }
 
 ::slotted(:is(img[slot='image'], picture[slot='image'])) {
   display: block;
   width: 100%;
   object-fit: cover;
   aspect-ratio: var(--sbb-lead-container-image-ratio);
+  border-radius: var(--sbb-lead-container-image-border-radius);
+}
 
-  @include sbb.mq($from: ultra) {
-    border-radius: var(--sbb-lead-container-border-radius);
-  }
+::slotted(:is(sbb-breadcrumb-group, sbb-block-link).sbb-lead-container-spacing) {
+  margin-block-end: var(--sbb-spacing-fixed-4x);
+}
+
+::slotted(sbb-title.sbb-lead-container-spacing) {
+  margin-block-start: 0;
+}
+
+::slotted(.sbb-lead-container-lead-text) {
+  margin-block: 0 var(--sbb-spacing-responsive-s);
 }
 
 .sbb-lead-container-image {
@@ -72,10 +81,10 @@
   border-radius: var(--sbb-lead-container-border-radius);
   padding-block: var(--sbb-lead-container-padding-block);
   background-color: var(--sbb-lead-container-background-color);
-  grid-column: 1/-1;
+  grid-column: 1 / -1;
 
   @include sbb.mq($from: medium) {
-    grid-column: 2/-2;
+    grid-column: 2 / -2;
   }
 
   @include sbb.mq($from: large) {
@@ -87,10 +96,10 @@
   }
 
   @include sbb.mq($from: wide) {
-    grid-column: 3/-3;
+    grid-column: 3 / -3;
   }
 
   @include sbb.mq($from: ultra) {
-    grid-column: 4/-4;
+    grid-column: 4 / -4;
   }
 }
diff --git a/src/components/lead-container/lead-container.snapshot.spec.ts b/src/components/lead-container/lead-container.snapshot.spec.ts
@@ -5,6 +5,7 @@ import {
   describeViewports,
   fixture,
   isVisualRegressionRun,
+  loadAssetAsBase64,
   testA11yTreeSnapshot,
   testVisualDiff,
   visualRegressionFixture,
@@ -18,6 +19,8 @@ import '../link/block-link/block-link.js';
 import '../title.js';
 import './lead-container.js';
 
+const leadImage = await loadAssetAsBase64(import.meta.resolve('./assets/lucerne.png'));
+
 describe(`sbb-lead-container`, () => {
   if (!isVisualRegressionRun()) {
     let element: SbbLeadContainerElement;
@@ -50,50 +53,33 @@ describe(`sbb-lead-container`, () => {
               html`
                 <sbb-lead-container>
                   <style>
-                    .inner-container {
-                      display: flex;
-                      flex-direction: column;
-                      gap: var(--sbb-spacing-fixed-4x);
-                    }
-                    sbb-title {
-                      margin-block-start: 0;
-                    }
-                    p.lead-text {
-                      margin-block: 0;
-                    }
                     p.other-content {
-                      margin-block-start: var(--sbb-spacing-responsive-s);
                       margin-block-end: 0;
                     }
                   </style>
-                  <img
-                    slot="image"
-                    src=${new URL('./assets/lucerne.png', import.meta.url).href}
-                    alt=""
-                  />
-                  <div class="inner-container">
-                    <sbb-breadcrumb-group>
-                      <sbb-breadcrumb
-                        href="#"
-                        icon-name="house-small"
-                        id="breadcrumb-0"
-                      ></sbb-breadcrumb>
-                      <sbb-breadcrumb href="#" id="breadcrumb-1">Level 1</sbb-breadcrumb>
-                      <sbb-breadcrumb href="#" id="breadcrumb-1">Level 2</sbb-breadcrumb>
-                      <sbb-breadcrumb href="#" id="breadcrumb-1">Level 3</sbb-breadcrumb>
-                      <sbb-breadcrumb href="#" id="breadcrumb-1">Level 4</sbb-breadcrumb>
-                    </sbb-breadcrumb-group>
-                    <sbb-block-link
-                      icon-placement="start"
-                      icon-name="chevron-small-left-small"
-                      size="xs"
-                      href="https://www.sbb.ch"
-                    >
-                      Link
-                    </sbb-block-link>
-                    <sbb-title>Title</sbb-title>
-                  </div>
-                  <p class="sbb-text-xl lead-text">
+                  <img slot="image" src=${leadImage} alt="" />
+                  <sbb-breadcrumb-group class="sbb-lead-container-spacing">
+                    <sbb-breadcrumb
+                      href="#"
+                      icon-name="house-small"
+                      id="breadcrumb-0"
+                    ></sbb-breadcrumb>
+                    <sbb-breadcrumb href="#" id="breadcrumb-1">Level 1</sbb-breadcrumb>
+                    <sbb-breadcrumb href="#" id="breadcrumb-1">Level 2</sbb-breadcrumb>
+                    <sbb-breadcrumb href="#" id="breadcrumb-1">Level 3</sbb-breadcrumb>
+                    <sbb-breadcrumb href="#" id="breadcrumb-1">Level 4</sbb-breadcrumb>
+                  </sbb-breadcrumb-group>
+                  <sbb-block-link
+                    icon-placement="start"
+                    icon-name="chevron-small-left-small"
+                    size="xs"
+                    href="https://www.sbb.ch"
+                    class="sbb-lead-container-spacing"
+                  >
+                    Link
+                  </sbb-block-link>
+                  <sbb-title class="sbb-lead-container-spacing">Title</sbb-title>
+                  <p class="sbb-text-xl sbb-lead-container-lead-text">
                     Lead text. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Integer enim
                     elit, ultricies in tincidunt quis, mattis eu quam. Nulla sit amet lorem
                     fermentum, molestie nunc ut, hendrerit risus.

diff --git a/src/components/lead-container/lead-container.stories.ts b/src/components/lead-container/lead-container.stories.ts
@@ -14,42 +14,29 @@ import readme from './readme.md?raw';
 const DefaultTemplate = (): TemplateResult => html`
   <sbb-lead-container>
     <style>
-      .inner-container {
-        display: flex;
-        flex-direction: column;
-        gap: var(--sbb-spacing-fixed-4x);
-      }
-      sbb-title {
-        margin-block-start: 0;
-      }
-      p.lead-text {
-        margin-block: 0;
-      }
       p.other-content {
-        margin-block-start: var(--sbb-spacing-responsive-s);
         margin-block-end: 0;
       }
     </style>
-    <sbb-image slot="image" image-src=${images[6]} aspect-ratio="free"></sbb-image>
-    <div class="inner-container">
-      <sbb-breadcrumb-group>
-        <sbb-breadcrumb href="#" icon-name="house-small" id="breadcrumb-0"></sbb-breadcrumb>
-        <sbb-breadcrumb href="#" id="breadcrumb-1">Level 1</sbb-breadcrumb>
-        <sbb-breadcrumb href="#" id="breadcrumb-1">Level 2</sbb-breadcrumb>
-        <sbb-breadcrumb href="#" id="breadcrumb-1">Level 3</sbb-breadcrumb>
-        <sbb-breadcrumb href="#" id="breadcrumb-1">Level 4</sbb-breadcrumb>
-      </sbb-breadcrumb-group>
-      <sbb-block-link
-        icon-placement="start"
-        icon-name="chevron-small-left-small"
-        size="xs"
-        href="https://www.sbb.ch"
-      >
-        Link
-      </sbb-block-link>
-      <sbb-title>Title</sbb-title>
-    </div>
-    <p class="sbb-text-xl lead-text">
+    <sbb-image slot="image" image-src=${images[6]}></sbb-image>
+    <sbb-breadcrumb-group class="sbb-lead-container-spacing">
+      <sbb-breadcrumb href="#" icon-name="house-small" id="breadcrumb-0"></sbb-breadcrumb>
+      <sbb-breadcrumb href="#" id="breadcrumb-1">Level 1</sbb-breadcrumb>
+      <sbb-breadcrumb href="#" id="breadcrumb-1">Level 2</sbb-breadcrumb>
+      <sbb-breadcrumb href="#" id="breadcrumb-1">Level 3</sbb-breadcrumb>
+      <sbb-breadcrumb href="#" id="breadcrumb-1">Level 4</sbb-breadcrumb>
+    </sbb-breadcrumb-group>
+    <sbb-block-link
+      icon-placement="start"
+      icon-name="chevron-small-left-small"
+      size="xs"
+      href="https://www.sbb.ch"
+      class="sbb-lead-container-spacing"
+    >
+      Link
+    </sbb-block-link>
+    <sbb-title class="sbb-lead-container-spacing">Title</sbb-title>
+    <p class="sbb-text-xl sbb-lead-container-lead-text">
       Lead text. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Integer enim elit,
       ultricies in tincidunt quis, mattis eu quam. Nulla sit amet lorem fermentum, molestie nunc ut,
       hendrerit risus.

diff --git a/src/components/lead-container/readme.md b/src/components/lead-container/readme.md
@@ -1,6 +1,9 @@
 The `sbb-lead-container` can be used for product pages to display a lead image and following content.
 
 Please note that the gaps between elements inside the unnamed slot (main content) need to be controlled by the consumer.
+However, consumers can use the predefined classes
+(`sbb-lead-container-lead-spacing` and `sbb-lead-container-lead-text`) to achieve a correct spacing.
+
 As per design rules, the spacings are defined as following:
 
 ```html
@@ -13,50 +16,37 @@ As per design rules, the spacings are defined as following:
 <p></p>
 <!-- var(--sbb-spacing-responsive-s) -->
 ...
+<!-- The last element should have zero block-end spacing to the container -->
 ```
 
-Full example with applied spacings in content:
+Full example with applied spacings (CSS classes) in content:
 
 ```html
 <sbb-lead-container>
   <style>
-    .inner-container {
-      display: flex;
-      flex-direction: column;
-      gap: var(--sbb-spacing-fixed-4x);
-    }
-    sbb-title {
-      margin-block-start: 0;
-    }
-    p.lead-text {
-      margin-block: 0;
-    }
     p.other-content {
-      margin-block-start: var(--sbb-spacing-responsive-s);
       margin-block-end: 0;
     }
   </style>
   <sbb-image
     slot="image"
     image-src="https://cdn.img.sbb.ch/content/dam/internet/lyne/Bahnhof-Luzern.jpg"
-    aspect-ratio="free"
   ></sbb-image>
-  <div class="inner-container">
-    <sbb-breadcrumb-group>
-      <sbb-breadcrumb href="#" icon-name="house-small" id="breadcrumb-0"></sbb-breadcrumb>
-      <sbb-breadcrumb href="#" id="breadcrumb-1">Level 1</sbb-breadcrumb>
-    </sbb-breadcrumb-group>
-    <sbb-block-link
-      icon-placement="start"
-      icon-name="chevron-small-left-small"
-      size="xs"
-      href="https://www.sbb.ch"
-    >
-      Link
-    </sbb-block-link>
-    <sbb-title>Title</sbb-title>
-  </div>
-  <p class="sbb-text-xl lead-text">Lead text.</p>
+  <sbb-breadcrumb-group class="sbb-lead-container-spacing">
+    <sbb-breadcrumb href="#" icon-name="house-small" id="breadcrumb-0"></sbb-breadcrumb>
+    <sbb-breadcrumb href="#" id="breadcrumb-1">Level 1</sbb-breadcrumb>
+  </sbb-breadcrumb-group>
+  <sbb-block-link
+    icon-placement="start"
+    icon-name="chevron-small-left-small"
+    size="xs"
+    href="https://www.sbb.ch"
+    class="sbb-lead-container-spacing"
+  >
+    Link
+  </sbb-block-link>
+  <sbb-title class="sbb-lead-container-spacing">Title</sbb-title>
+  <p class="sbb-text-xl sbb-lead-container-lead-text">Lead text.</p>
   <p class="sbb-text-m other-content">Other content.</p>
 </sbb-lead-container>
 ```