fix(grid): include padding in max-width and align with nav bar (#1120)

Relates to #867,
#867 (comment)

After a talk with @jannick-ux we came to the conclusion that:
- width in Figma are meant to include the padding so if grid max width
is 1440px, the actual component width is 1312px so in total: 1312px
width + 2 * 64px padding = 1440px.

What I did:
- fix grid to use `box-sizing: border-box;` instead of `content-box`
- add screenshot tests which shows a full app layout with nav bar and
page content to test that both are aligned with each other
- add grid demo page to the alpha demo app so we can play around with it
and see it in action

.changeset/brown-readers-kneel.md

@@ -0,0 +1,5 @@
+---
+"sit-onyx": patch
+---
+
+fix(grid): include padding in max-width

apps/alpha-test-app/package.json

@@ -12,6 +12,7 @@
   "dependencies": {
     "@sit-onyx/icons": "workspace:^",
     "@vueuse/core": "^10.9.0",
+    "pinia": "^2.1.7",
     "sit-onyx": "workspace:^",
     "vue": "^3.4.27",
     "vue-i18n": "^9.13.1",

apps/alpha-test-app/src/App.vue

@@ -13,13 +13,16 @@ import {
 } from "sit-onyx";
 import { RouterView, useRouter } from "vue-router";
 import onyxLogo from "./assets/onyx-logo.svg";
+import { useGridStore } from "./stores/grid-store";
 
 const router = useRouter();
+const gridStore = useGridStore();
 
 const navItems = [
   { label: "Home", href: "/" },
   { label: "Form Demo", href: "/form-demo" },
   { label: "Layout Demo", href: "/layout-demo" },
+  { label: "Grid Demo", href: "/grid" },
 ] satisfies OnyxNavItemProps[];
 
 const userMenuOptions = [
@@ -32,7 +35,12 @@ const toggleDark = useToggle(isDark);
 </script>
 
 <template>
-  <OnyxAppLayout>
+  <OnyxAppLayout
+    :class="{
+      'onyx-grid-max-md': gridStore.isMaxWidth,
+      'onyx-grid-center': gridStore.isCentered,
+    }"
+  >
     <template #navBar>
       <OnyxNavBar
         app-name="Alpha Test App"

apps/alpha-test-app/src/main.ts

@@ -1,3 +1,4 @@
+import { createPinia } from "pinia";
 import { createOnyx } from "sit-onyx";
 import onyxDeDE from "sit-onyx/locales/de-DE.json";
 import onyxKoKR from "sit-onyx/locales/ko-KR.json";
@@ -30,7 +31,7 @@ async function setupApp() {
     routes,
   });
 
-  const app = createApp(App).use(i18n).use(router).use(onyx);
+  const app = createApp(App).use(i18n).use(router).use(onyx).use(createPinia());
   await router.isReady();
 
   app.mount("#app");

apps/alpha-test-app/src/router/index.ts

@@ -18,4 +18,9 @@ export const routes: RouteRecordRaw[] = [
     name: "layoutDemo",
     component: () => import("../views/LayoutDemoView.vue"),
   },
+  {
+    path: "/grid",
+    name: "gridDemo",
+    component: () => import("../views/GridDemo.vue"),
+  },
 ];

apps/alpha-test-app/src/stores/grid-store.ts

@@ -0,0 +1,9 @@
+import { defineStore } from "pinia";
+import { ref } from "vue";
+
+export const useGridStore = defineStore("grid", () => {
+  const isMaxWidth = ref(false);
+  const isCentered = ref(false);
+
+  return { isMaxWidth, isCentered };
+});

apps/alpha-test-app/src/views/GridDemo.vue

@@ -0,0 +1,33 @@
+<script lang="ts" setup>
+import { OnyxPageLayout, OnyxSwitch } from "sit-onyx";
+import { useGridStore } from "../stores/grid-store";
+
+const gridStore = useGridStore();
+</script>
+
+<template>
+  <OnyxPageLayout>
+    <div class="onyx-grid">
+      <OnyxSwitch
+        v-model="gridStore.isMaxWidth"
+        class="onyx-grid-span-3"
+        label="Is max width (md)"
+      />
+      <OnyxSwitch
+        v-if="gridStore.isMaxWidth"
+        v-model="gridStore.isCentered"
+        class="onyx-grid-span-3"
+        label="Is centered"
+      />
+
+      <p class="content onyx-grid-span-16">Page content</p>
+    </div>
+  </OnyxPageLayout>
+</template>
+
+<style lang="scss" scoped>
+.content {
+  background-color: var(--onyx-color-base-info-200);
+  color: var(--onyx-color-text-icons-info-bold);
+}
+</style>

apps/docs/src/development/grid.md

@@ -7,7 +7,7 @@ This library is currently in early / active development.
 The grid implementation in **onyx** differs from other design system implementations that you might know.
 Instead of having a fixed 12 column grid, we have a dynamic grid with 4, 8, 12, 16 and optionally 20 columns.
 The amount of columns differs based on the active breakpoint.
-You can find a detailed description of the system in the design system docs (TODO: Add link to grid UX definition).
+You can find a detailed description of the system in the [design system docs](/basics/breakpoints-grid).
 
 ::: info
 The dynamic grid has the following advantages over a fixed column grid:
@@ -21,7 +21,8 @@ The dynamic grid has the following advantages over a fixed column grid:
 Grid elements span the number of columns that is assigned to them.
 If there are less columns available than an element is assigned, it will span all columns of the row.
 
-Additionally, an element can also be configured to span a specific amount of columns for a minimum breakpoint (TODO: Add link to breakpoint UX definition).
+Additionally, an element can also be configured to span a specific amount of columns for a minimum breakpoint.
+To learn about the grid breakpoints, please refer to the [design system docs](/basics/breakpoints-grid#breakpoints).
 Multiple span definitions can then be combined to resize an element based on the breakpoint.
 
 ## Example

packages/sit-onyx/src/components/OnyxNavBar/OnyxNavBar.ct.tsx

@@ -6,6 +6,8 @@ import {
   executeMatrixScreenshotTest,
 } from "../../playwright/screenshots";
 import { ONYX_BREAKPOINTS } from "../../types";
+import OnyxAppLayout from "../OnyxAppLayout/OnyxAppLayout.vue";
+import OnyxPageLayout from "../OnyxPageLayout/OnyxPageLayout.vue";
 import OnyxUserMenu from "../OnyxUserMenu/OnyxUserMenu.vue";
 
 test.beforeEach(async ({ page }) => {
@@ -39,48 +41,6 @@ test.describe("Screenshot tests", () => {
       ),
     });
   }
-
-  executeMatrixScreenshotTest({
-    name: "Navigation bar (lg, grid max-width md)",
-    columns: ["default"],
-    rows: ["default", "centered"],
-    // TODO: remove when contrast issues are fixed in https://github.com/SchwarzIT/onyx/issues/410
-    disabledAccessibilityRules: ["color-contrast"],
-    component: (column, row) => (
-      <OnyxNavBar
-        style={{
-          width: `${ONYX_BREAKPOINTS.lg}px`,
-          "--onyx-grid-max-width": `${ONYX_BREAKPOINTS.md}px`,
-        }}
-        class={{ "onyx-grid-center": row === "centered" }}
-        appName="App name"
-        logoUrl={MOCK_PLAYWRIGHT_LOGO_URL}
-        withBackButton={row.includes("back")}
-      >
-        <OnyxNavItem label="Item" active />
-        <OnyxNavItem label="Item" />
-
-        {row.includes("context") && (
-          <template v-slot:contextArea>
-            <OnyxUserMenu username="John Doe" options={[]} />
-          </template>
-        )}
-      </OnyxNavBar>
-    ),
-    beforeScreenshot: async (component, page) => {
-      // colorize background so the max-width can be seen easily
-      await page.addStyleTag({
-        content: `
-          .onyx-nav-bar {
-            background-color: var(--onyx-color-base-danger-200);
-          }
-          .onyx-nav-bar__content {
-            background-color: var(--onyx-color-base-background-blank);
-          }
-        `,
-      });
-    },
-  });
 });
 
 test("should behave correctly", async ({ mount }) => {
@@ -112,3 +72,46 @@ test("should behave correctly", async ({ mount }) => {
 
   await expect(component.getByRole("button", { name: "Custom app area" })).toBeVisible();
 });
+
+test("should be aligned with the grid in a full app layout", async ({ page, mount }) => {
+  await defineLogoMockRoutes(page);
+  await page.setViewportSize({ width: ONYX_BREAKPOINTS.xl, height: 400 });
+
+  await page.addStyleTag({
+    content: "body { margin: 0; }",
+  });
+
+  await mount(
+    <OnyxAppLayout>
+      <OnyxNavBar appName="App name" logoUrl={MOCK_PLAYWRIGHT_LOGO_URL}>
+        <OnyxNavItem label="Item" active />
+        <OnyxNavItem label="Item" />
+
+        <template v-slot:contextArea>
+          <OnyxUserMenu username="John Doe" options={[]} />
+        </template>
+      </OnyxNavBar>
+
+      <OnyxPageLayout>
+        <div class="onyx-grid">
+          <div
+            class="onyx-grid-span-16"
+            style={{ backgroundColor: "var(--onyx-color-base-info-200)" }}
+          >
+            Page content...
+          </div>
+        </div>
+      </OnyxPageLayout>
+    </OnyxAppLayout>,
+  );
+
+  await expect(page).toHaveScreenshot("grid-default.png");
+
+  const app = page.locator(".onyx-app");
+
+  await app.evaluate((element) => element.classList.add("onyx-grid-max-md"));
+  await expect(page).toHaveScreenshot("grid-max-width.png");
+
+  await app.evaluate((element) => element.classList.add("onyx-grid-center"));
+  await expect(page).toHaveScreenshot("grid-max-center.png");
+});

packages/sit-onyx/src/components/OnyxNavBar/OnyxNavBar.vue

@@ -76,6 +76,8 @@ const { t } = injectI18n();
 
 .onyx-nav-bar {
   @include layers.component() {
+    --padding-inline: var(--onyx-spacing-3xl);
+
     border-bottom: var(--onyx-1px-in-rem) solid var(--onyx-color-base-neutral-300);
     background-color: var(--onyx-color-base-background-blank);
     font-family: var(--onyx-font-family);
@@ -89,18 +91,18 @@ const { t } = injectI18n();
       align-items: center;
       gap: var(--onyx-spacing-md);
       height: 100%;
-      padding: 0 var(--onyx-spacing-3xl);
+      padding-inline: var(--padding-inline);
 
       // sync with grid
       max-width: var(--onyx-grid-max-width);
       margin-inline: var(--onyx-grid-margin-inline);
 
       @include breakpoints.container(max, sm) {
-        padding: 0 var(--onyx-spacing-xl);
+        --padding-inline: var(--onyx-spacing-xl);
       }
 
       @include breakpoints.container(max, xs) {
-        padding: 0 var(--onyx-spacing-md);
+        --padding-inline: var(--onyx-spacing-md);
       }
     }
 

packages/sit-onyx/src/styles/grid.ct.tsx

@@ -191,10 +191,7 @@ MAX_WIDTH_TEST_SETUP.forEach(({ breakpoint, className }) => {
     const EXPECTED_LEFT = 0;
     expect(box.left).toBe(EXPECTED_LEFT);
     const BREAKPOINT_MAX = ONYX_BREAKPOINTS[breakpoint];
-    // We need to add the margin as it is implemented via a padding which isn't included in the boundingclientrect
-    const MARGIN = 64;
-    const EXPECTED_RIGHT = 2 * MARGIN + BREAKPOINT_MAX;
-    expect(box.right).toBe(EXPECTED_RIGHT);
+    expect(box.right).toBe(BREAKPOINT_MAX);
   });
 });
 
@@ -219,9 +216,7 @@ MAX_WIDTH_TEST_SETUP.forEach(({ breakpoint, className }) => {
       .evaluateHandle((el) => el.getBoundingClientRect())
       .then((res) => res.jsonValue());
 
-    // We need to add the margin as it is implemented via a padding which isn't included in the boundingclientrect
-    const MARGIN = 64;
-    const BOX_MAX_WIDTH = ONYX_BREAKPOINTS[breakpoint] + 2 * MARGIN;
+    const BOX_MAX_WIDTH = ONYX_BREAKPOINTS[breakpoint];
     const EXPECTED_LEFT = (VIEWPORT_WIDTH - BOX_MAX_WIDTH) / 2;
     expect(box.left).toBe(EXPECTED_LEFT);
     const EXPECTED_RIGHT = EXPECTED_LEFT + BOX_MAX_WIDTH;

packages/sit-onyx/src/styles/grid.scss

@@ -22,7 +22,7 @@ $gridVariants: (
     // Its called margin in the UX concept. So we keep that name for consistency, even when it is implemented through a padding.
     padding: var(--onyx-grid-margin);
     // often web projects override the default box-sizing, but it is important for the max-width and alignment to work correctly
-    box-sizing: content-box;
+    box-sizing: border-box;
     max-width: var(--onyx-grid-max-width, none);
     margin-inline: var(--onyx-grid-margin-inline);
   }