Add option to disable automatic media sizing in River.Visual (#59)

Author: rezrah

.changeset/hungry-pillows-watch.md

@@ -0,0 +1,11 @@
+---
+'@primer/react-brand': patch
+---
+
+Added `fillMedia` prop to River.Visual
+
+Previously, all image and video children were automatically styled to fit the parent elements dimensions. 
+
+This isn't ideal in all situations, so the ability to toggle-off this behaviour is now available through an `fillMedia` prop.
+
+

.github/workflows/ui_test_primitives_diff.yml

@@ -45,7 +45,7 @@ jobs:
 
       - name: Export diff
         run: |
-          diff -y --width=150 -B -I '*' -I 'Generated' --suppress-common-lines temp-main/lib/css lib/css  | tail -n +2 > diff.txt
+          diff -y --width=150 -B -I '*' -I 'Generated' --suppress-common-lines temp-main/lib/design-tokens/css lib/design-tokens/css  | tail -n +2 > diff.txt
           npx ts-node ./scripts/ci-check-diff.ts
 
       - name: Read raw diff

.gitignore

@@ -9,6 +9,7 @@ storybook-static
 .test/
 coverage
 playwright-test-results/
+playwright-report/
 
 # misc
 .DS_Store

docs/content/components/River.mdx

@@ -159,10 +159,11 @@ import {River} from '@primer/react-brand'
 
 ### River.Visual <Label>Required</Label>
 
-| Name        | Type           | Default | Description                                                                                                      |
-| :---------- | :------------- | :-----: | :--------------------------------------------------------------------------------------------------------------- |
-| `children`  | `ReactElement` |         | Bring your own component (BYOC) `img` or `ReactElement` (E.g. Next.js `Image` component)                         |
-| `hasShadow` | `boolean`      | `true`  | Shadow automatically applied to the `children`. Set be `false` when the child node has a transparent background. |
+| Name        | Type           | Default | Description                                                                                                                                   |
+| :---------- | :------------- | :-----: | :-------------------------------------------------------------------------------------------------------------------------------------------- |
+| `fillMedia` | `boolean`      |  true   | Automatically styles images and video to fill and fit the width of the parent. Disable this setting if you have bespoke styling requirements. |
+| `children`  | `ReactElement` |         | Bring your own component (BYOC) `img` or `ReactElement` (E.g. Next.js `Image` component)                                                      |
+| `hasShadow` | `boolean`      | `true`  | Shadow automatically applied to the `children`. Set be `false` when the child node has a transparent background.                              |
 
 ### River.Content <Label>Required</Label>
 

package.json

@@ -42,9 +42,9 @@
     "start:docs": "cd docs && npm run develop",
     "start:css-types": "tcm -w -p 'src/**/*.css' .",
     "test": "jest",
-    "test:visual": "scripts/playwright/run-visual-tests",
+    "test:visual": "npx build-storybook && scripts/playwright/run-visual-tests",
     "test:visual:generate": "npm run build:docs && ts-node scripts/playwright/playwright.generate-tests.ts",
-    "test:visual:update-snapshots": "scripts/playwright/update-visual-snapshots"
+    "test:visual:update-snapshots": "npx build-storybook && scripts/playwright/update-visual-snapshots"
   },
   "peerDependencies": {
     "react": ">=17 <= 18",

scripts/playwright/playwright.generate-tests.ts

@@ -33,10 +33,17 @@
    * Manual lookup for tests that need animation or side-effects to complete before tests start
    */
   const waitForTimeoutLookup = {
-    'components-faq-fixtures--all-open': 250, // for the animation
-    'components-river--video': 1000 // video is slow to load
+    'components-faq-fixtures--all-open': 250 // for the animation
   }
 
+  /**
+   * Manual lookup for tests that we want to skip
+   * Only add tests here that aren't suitable for visual regression testing
+   */
+  const skipTestLookup = [
+    'components-river--video' // video makes this too flakey
+  ]
+
   const categorisedStories = Object.keys(stories as Stories).reduce((acc, key) => {
     const {id, story: storyName, importPath} = stories[key]
 
@@ -79,6 +86,9 @@
     test.describe('Visual Comparison: ${key}', () => {
   
       ${componentStories.reduce((acc, {id, storyName, timeout}) => {
+        if (skipTestLookup.includes(id)) {
+          return acc
+        }
         return (acc += `
           test('${storyName}', async ({page}) => {
               await page.goto('http://localhost:${port}/iframe.html?args=&id=${id}&viewMode=story')

src/FAQ/FAQ.visual.spec.ts-snapshots/Visual-Comparison-FAQ-Groups-1-linux.png

@@ -5,17 +5,17 @@
   margin: var(--brand-River-layout-margin-vertical);
 }
 
+.River--align-center {
+  text-align: center;
+}
+
 .River__visual {
   position: relative;
   order: 2;
 }
 
-.River--align-center {
-  text-align: center;
-}
-
-.River__visual img,
-.River__visual video {
+.River__visual--fill-media img,
+.River__visual--fill-media video {
   width: 100%;
   height: 100%;
   object-fit: cover;

src/River/River.module.css

@@ -1,7 +1,8 @@
 declare const styles: {
   readonly "River": string;
-  readonly "River__visual": string;
   readonly "River--align-center": string;
+  readonly "River__visual": string;
+  readonly "River__visual--fill-media": string;
   readonly "River__visual--has-shadow": string;
   readonly "River__heading": string;
   readonly "River__call-to-action": string;

src/River/River.module.css.d.ts

@@ -0,0 +1,33 @@
+.overflow {
+  overflow: hidden;
+}
+
+.container {
+  max-width: 1280px;
+  margin: 0 auto;
+}
+
+.river-logos {
+  display: grid;
+  width: 120%;
+  grid-template: auto/repeat(7, 1fr);
+  grid-gap: 1rem;
+  gap: 1rem;
+  align-self: center;
+  margin-right: -20%;
+  max-width: 600px;
+}
+
+.river-logo {
+  background: var(--brand-canvas-default);
+  box-shadow: rgba(140, 149, 159, 0.2) 0px 8px 24px 0px;
+  height: auto;
+}
+
+.river-logo--row-two {
+  margin-left: 50%;
+}
+
+.river-logo--row-three {
+  margin-left: 100%;
+}