carbon-design-system / carbon-components-vue

Vue implementation of the Carbon Design System
http://vue.carbondesignsystem.com
Apache License 2.0
608 stars 178 forks source link

docs: cv-file-uploader docs enhancements #1520

Closed felipebritor closed 11 months ago

felipebritor commented 1 year ago

Contributes to #1519

What did you do?

 Why did you do it?

How have you tested it?

Were docs updated if needed?

felipebritor commented 1 year ago

Hi, @davidnixon . Forgive me, again, for the tardiness.

I need your feedback on a few things:

  1. I'm persisting the files uploaded in the storybook file as the storybook component is refreshed on some control changes. Unless I'm doing something wrong, as not all control changes reset the component, looks like this is the way to go.
  2. setInvalidMessage and setState are working following the example at react's storybook. Unfortunately, for clear and remove exposed methods, the native control types doesn't not offer a good option for them. There is an old issue at storybook repo requesting a button control type but not progress. One of the comments suggested a workaround which does the trick, but it's quite hacky and fragile. So I need your guidance if it makes sense to use it. We'll have to be mindful of: storybook version must be locked, keep aware that each new version update might break it and update accordingly (even making it look for a index-hash.js file does not seems stable enough but might be better than leaving it hardcoded) You can check it at f63ad8a. If you think it's better not to follow this approach, I can see other options. image
  3. The 'loading' state icon used together with the invalid one is broken (image 2). It's also broken in the v2 and the react version only display a single icon. Should I follow react or adjust the css? image

edit: this PR now is based on #1529, new commits starts at 4ddb35a

felipebritor commented 11 months ago

Hi, @davidnixon, just a friendly reminder about the points I need your input ☝️

davidnixon commented 11 months ago

@felipebritor oh sorry totally missed this. I will update today!

davidnixon commented 11 months ago
  1. I'm persisting the files uploaded in the storybook file as the storybook component is refreshed on some control changes. Unless I'm doing something wrong, as not all control changes reset the component, looks like this is the way to go.

This looks right to me. I know some changes cause the storybook to reload but I'm not sure why. I thought I might take a closer look at it Storybook 7.

  1. setInvalidMessage and setState are working following the example at react's storybook. ...

The clear and remove button in the storybook work correctly for me but I think you are concerned that the control type "function" is not supported?

  clear: {
    control: 'none',
    type: 'function',
    table: {
      type: { summary: '() => void' },
      category: 'exposed methods',
    },
    description: 'Clear file list',
  },
  1. The 'loading' state icon used together with the invalid one is broken ...

I think it should be like the react storybook. Only 1 icon is allowed.

felipebritor commented 11 months ago

The clear and remove button in the storybook work correctly for me but I think you are concerned that the control type "function" is not supported?

function would be the type and button the control type. This control type does not exist and is set as feature request for some time now. What I did was setup a "hacky" custom button type at 1e28477.

I think it should be like the react storybook. Only 1 icon is allowed.

Done at ff4241f 👍