Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating docs for BaseImage and BaseImagePlaceholder #15

Merged
merged 4 commits into from
Sep 22, 2021

Conversation

stephiescastle
Copy link
Member

  • adds lazy loading stories for BaseImage and BaseImagePlaceholder
  • fixes storyDecorator html root for AnimationCaret. The html root was specified for the component, which meant every story would need to use the decorator for the html markup add on to work properly. This change limits the html root parameter to stories that use a decorator.

To Test

  1. npm i if you haven't already
  2. npm run storybook
  3. Check the BaseImage and BaseImagePlaceholder stories. Their content should cover the original docs from www-frontend, unless it was Vue-specific.

Caveats

When testing, you will notice that the documentation is not an exact copy of that from www-frontend. This is due to <ArgsTable /> not working as expected in mdx files in this repo. When using ArgsTable, an error of "Args not valid" would appear. Oddly, the automatically generated ArgsTable works just fine when not creating a custom docs page via MDX. I figured a working ArgsTable would be preferred, so I stuck with using automatically generated docs pages (we used custom docs pages in www-frontend, which is why I went down this path).

To make up the differences, I added field descriptions to the argsTable and also marked required fields. I also decided some documentation from the original www-frontend wasn't necessary, as it was evident in the dropdown menus (i.e. the list of object fit classes and aspect ratio classes)

Documented below is how we would implement mdx files if ArgsTable was working (for future reference!):

Expand MDX method example
  1. Remove the default export from BaseImage.stories.js
// BaseImage.stories.js
import { BaseImageTemplate } from './BaseImage.js'

export const BaseImageData = {
  src: 'https://picsum.photos/800/400',
  srcset: 'https://picsum.photos/800/400 800w, ',
  alt: 'Alt text for image',
  width: '800',
  height: '400',
  loading: 'lazy',
  imageClass: '',
  objectFitClass: 'object-contain',
}
export const Default = BaseImageTemplate.bind({})
Default.args = BaseImageData

export const LazyLoading = BaseImageTemplate.bind({})
LazyLoading.args = BaseImageData
LazyLoading.decorators = [
  (Story) => `
  <div class="max-w-full">
    <div style="height:2500px">
      Scroll down
    </div>
    <div id="storyDecorator">
      ${Story()}
    </div>
  </div>
  `,
]
LazyLoading.parameters = {
  html: {
    root: '#storyDecorator',
  },
}
  1. Add BaseImage.stories.mdx to the same folder:
// BaseImage.stories.mdx
import { Meta, Story, Canvas, ArgsTable } from '@storybook/addon-docs'
import * as stories from './BaseImage.stories.js'
import { BaseImageTemplate } from './BaseImage.js'

<Meta
  title="Components/Base/BaseImage"
  component={BaseImageTemplate}
  parameters={{
    docs: {
      iframeHeight: 500,
    },
  }}
  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: [
          'object-none',
          'object-contain',
          'object-cover',
          'object-fill',
          'object-scale-down',
        ],
      },
      table: {
        defaultValue: { summary: 'object-contain' },
      },
    },
    imageClass: {
      type: 'string',
      description: 'Apply any CSS class directly to the image element',
    },
  }}
/>

# BaseImage

The BaseImage component is a simple `<img />` tag wrapped in a `<div>` and is used to render an image with object-fit classes and lazy loading properties.

<Canvas>
  <Story story={stories.Default} />
</Canvas>

## Props

<ArgsTable of={BaseImageTemplate} />

## Lazy Loading Demo

<Canvas>
  <Story story={stories.LazyLoading} />
</Canvas>

Copy link
Member

@Scotchester Scotchester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Just one tiny suggestion.

stephiescastle and others added 2 commits September 22, 2021 10:19
…ceholder.stories.js

Co-authored-by: Scott Cranfill <scott@scottcranfill.com>
…s no effectand actually shouldn't be changed!
@stephiescastle
Copy link
Member Author

stephiescastle commented Sep 22, 2021

I made a small change that removes the loading attribute from BaseImage's ArgsTable since it really shouldn't be changed. I opened a new issue to address lazy vs. eager and feature detection at a later time: #19

@stephiescastle stephiescastle merged commit 6ec911c into main Sep 22, 2021
@stephiescastle stephiescastle deleted the fix/14-base-image-stories branch September 22, 2021 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants