From f856c664b567d617a3bb29f150026a9fcb52d858 Mon Sep 17 00:00:00 2001 From: Stephanie Smith Date: Tue, 21 Sep 2021 11:18:16 -0700 Subject: [PATCH 1/4] adding more docs to BaseImage and BaseImagePlaceholder, fixing html root parameter in AnimationCaret --- .../AnimationCaret/AnimationCaret.stories.js | 8 ++- .../components/BaseImage/BaseImage.stories.js | 63 ++++++++++++++++--- .../BaseImagePlaceholder.stories.js | 49 ++++++++++++--- 3 files changed, 101 insertions(+), 19 deletions(-) diff --git a/storybook/stories/components/AnimationCaret/AnimationCaret.stories.js b/storybook/stories/components/AnimationCaret/AnimationCaret.stories.js index 6e783ea4..862cd685 100644 --- a/storybook/stories/components/AnimationCaret/AnimationCaret.stories.js +++ b/storybook/stories/components/AnimationCaret/AnimationCaret.stories.js @@ -20,9 +20,6 @@ export default { }, parameters: { viewMode: 'docs', - html: { - root: '#storyDecorator', // omit decorators from html output - }, docs: { description: { component: @@ -40,3 +37,8 @@ Inline.args = { text: 'Longer text to demonstrate text wrap', inline: true } Inline.decorators = [ (Story) => `
${Story()}
`, ] +Inline.parameters = { + html: { + root: '#storyDecorator', + }, +} diff --git a/storybook/stories/components/BaseImage/BaseImage.stories.js b/storybook/stories/components/BaseImage/BaseImage.stories.js index 986d187f..fcdf17a1 100644 --- a/storybook/stories/components/BaseImage/BaseImage.stories.js +++ b/storybook/stories/components/BaseImage/BaseImage.stories.js @@ -4,8 +4,35 @@ export default { title: 'Components/Base/BaseImage', excludeStories: /.*Data$/, argTypes: { + src: { + type: { name: 'string', required: true }, + }, + srcset: { + type: 'string', + }, + alt: { + type: { name: 'string', required: true }, + }, + width: { + type: { name: 'number', required: true }, + }, + height: { + type: { name: 'number', required: true }, + }, + loading: { + type: 'string', + control: { + type: 'select', + options: ['lazy', 'eager'], + }, + table: { + defaultValue: { summary: 'lazy' }, + }, + }, objectFitClass: { type: 'string', + description: + 'Use TailwindCSS object fit classes to specify how the image will scale within `BaseImagePlaceholder`', control: { type: 'select', options: [ @@ -20,22 +47,16 @@ export default { defaultValue: { summary: 'object-contain' }, }, }, - loading: { + imageClass: { type: 'string', - control: { - type: 'select', - options: ['lazy', 'eager'], - }, - table: { - defaultValue: { summary: 'lazy' }, - }, + description: 'Apply any CSS class directly to the image element', }, }, parameters: { docs: { description: { component: - 'The BaseImage component is a simple `` tag used to embed an image with object-fit classes and lazy loading properties.', + 'The BaseImage component is a simple `` tag wrapped in a `
` and is used to render an image with object-fit classes and lazy loading properties.', }, }, }, @@ -53,3 +74,27 @@ export const BaseImageData = { } export const Default = BaseImageTemplate.bind({}) Default.args = BaseImageData + +export const LazyLoading = BaseImageTemplate.bind({}) +LazyLoading.args = BaseImageData +LazyLoading.decorators = [ + (Story) => ` +
+
+ Scroll down +
+
+ ${Story()} +
+
+ `, +] +LazyLoading.parameters = { + html: { + root: '#storyDecorator', + }, + docs: { + storyDescription: + '`class: -light` -- can be combined with any variant for light blue colors', + }, +} diff --git a/storybook/stories/components/BaseImagePlaceholder/BaseImagePlaceholder.stories.js b/storybook/stories/components/BaseImagePlaceholder/BaseImagePlaceholder.stories.js index cb779d2a..18fa3381 100644 --- a/storybook/stories/components/BaseImagePlaceholder/BaseImagePlaceholder.stories.js +++ b/storybook/stories/components/BaseImagePlaceholder/BaseImagePlaceholder.stories.js @@ -5,6 +5,8 @@ export default { argTypes: { aspectRatio: { type: 'string', + description: + 'Aspect ratio CSS class. View dropdown to see all options. More classes can be added in `/src/scss/_aspect-ratios.scss`', control: { type: 'select', options: [ @@ -32,6 +34,7 @@ export default { }, theme: { type: 'string', + description: 'Theme color for the placeholder background.', control: { type: 'select', options: ['light-theme', 'dark-theme', 'transparent-theme'], @@ -42,9 +45,12 @@ export default { }, noLogo: { type: 'boolean', + description: 'If a JPL logo should appear when there is no image', }, objectFitClass: { type: 'string', + description: + "Apply a TailwindCSS object fit class to `BaseImage` to specify how the image will scale within the placeholder's aspect ratio.", control: { type: 'select', options: [ @@ -63,13 +69,17 @@ export default { parameters: { docs: { description: { - component: `The \`BaseImagePlaceholder\` component is designed to appear as a temporary stand-in to be replaced by an actual image.
- `, + component: `The \`BaseImagePlaceholder\` component is designed to appear as a temporary stand-in to be replaced by an actual image. + +- expects to contain an image as a child element in its primary slot +- provides a lazy-loading block for the image to load into +- can be used to maintain an aspect ratio +- compatible with TailwindCSS classes, e.g. \`.rounded-lg\` to have round edges around the image. Think of it like a frame to put an image within. + +## Accessibility notes + +BaseImagePlaceholder is a presentational element consisting of a single \`div\` with a background image (JPL Logo), without semantic meaning, it simply prevents page load becoming janky by setting a 'placeholder' for the images that are yet to be loaded with loading="lazy" or LazySizes fallback. As such it should not need to meet color contrast requirements. + `, }, }, }, @@ -85,3 +95,28 @@ NoImage.args = { noLogo: false, noImage: true, } +export const LazyLoading = BaseImagePlaceholderTemplate.bind({}) +LazyLoading.args = { + noLogo: false, +} +LazyLoading.decorators = [ + (Story) => ` +
+
+ Scroll down +
+
+ ${Story()} +
+
+ `, +] +LazyLoading.parameters = { + html: { + root: '#storyDecorator', + }, + docs: { + storyDescription: + "`BaseImagePlaceholder` is compatible with `BaseImage`'s lazy loading behavior.", + }, +} From bde2714852742ea89170ff65773a5cb7b99a09c8 Mon Sep 17 00:00:00 2001 From: Stephanie Smith Date: Tue, 21 Sep 2021 11:32:03 -0700 Subject: [PATCH 2/4] removing misplaced description --- storybook/stories/components/BaseImage/BaseImage.stories.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/storybook/stories/components/BaseImage/BaseImage.stories.js b/storybook/stories/components/BaseImage/BaseImage.stories.js index fcdf17a1..40954799 100644 --- a/storybook/stories/components/BaseImage/BaseImage.stories.js +++ b/storybook/stories/components/BaseImage/BaseImage.stories.js @@ -93,8 +93,4 @@ LazyLoading.parameters = { html: { root: '#storyDecorator', }, - docs: { - storyDescription: - '`class: -light` -- can be combined with any variant for light blue colors', - }, } From 55bc35d3249b42cfe553993ea9c632eb58dffb7d Mon Sep 17 00:00:00 2001 From: Stephanie Smith Date: Wed, 22 Sep 2021 10:19:05 -0700 Subject: [PATCH 3/4] Update storybook/stories/components/BaseImagePlaceholder/BaseImagePlaceholder.stories.js Co-authored-by: Scott Cranfill --- .../BaseImagePlaceholder/BaseImagePlaceholder.stories.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storybook/stories/components/BaseImagePlaceholder/BaseImagePlaceholder.stories.js b/storybook/stories/components/BaseImagePlaceholder/BaseImagePlaceholder.stories.js index 18fa3381..f98c1cb6 100644 --- a/storybook/stories/components/BaseImagePlaceholder/BaseImagePlaceholder.stories.js +++ b/storybook/stories/components/BaseImagePlaceholder/BaseImagePlaceholder.stories.js @@ -78,7 +78,7 @@ export default { ## Accessibility notes -BaseImagePlaceholder is a presentational element consisting of a single \`div\` with a background image (JPL Logo), without semantic meaning, it simply prevents page load becoming janky by setting a 'placeholder' for the images that are yet to be loaded with loading="lazy" or LazySizes fallback. As such it should not need to meet color contrast requirements. +BaseImagePlaceholder is a presentational element consisting of a single \`div\` with a background image (JPL Logo), without semantic meaning, it simply prevents page load becoming janky by setting a 'placeholder' for the images that are yet to be loaded with \`loading="lazy"\` or LazySizes fallback. As such it should not need to meet color contrast requirements. `, }, }, From 08cd4034f3457ea54080c94551eb0a7b6dcab895 Mon Sep 17 00:00:00 2001 From: Stephanie Smith Date: Wed, 22 Sep 2021 14:30:56 -0700 Subject: [PATCH 4/4] removing the loading attribute from BaseImage's argsTable since it has no effectand actually shouldn't be changed! --- storybook/stories/components/BaseImage/BaseImage.js | 4 +--- .../stories/components/BaseImage/BaseImage.stories.js | 11 ----------- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/storybook/stories/components/BaseImage/BaseImage.js b/storybook/stories/components/BaseImage/BaseImage.js index 19a63c3b..7c19b91f 100644 --- a/storybook/stories/components/BaseImage/BaseImage.js +++ b/storybook/stories/components/BaseImage/BaseImage.js @@ -4,11 +4,9 @@ export const BaseImageTemplate = ({ alt, width, height, - loading, imageClass, objectFitClass, }) => { - if (!loading) loading = 'lazy' return `
${alt}
` } diff --git a/storybook/stories/components/BaseImage/BaseImage.stories.js b/storybook/stories/components/BaseImage/BaseImage.stories.js index 40954799..de5d5eeb 100644 --- a/storybook/stories/components/BaseImage/BaseImage.stories.js +++ b/storybook/stories/components/BaseImage/BaseImage.stories.js @@ -19,16 +19,6 @@ export default { height: { type: { name: 'number', required: true }, }, - loading: { - type: 'string', - control: { - type: 'select', - options: ['lazy', 'eager'], - }, - table: { - defaultValue: { summary: 'lazy' }, - }, - }, objectFitClass: { type: 'string', description: @@ -68,7 +58,6 @@ export const BaseImageData = { alt: 'Alt text for image', width: '800', height: '400', - loading: 'lazy', imageClass: '', objectFitClass: 'object-contain', }