carbon-design-system / carbon-components-vue

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

feat(cv-text-input-skeleton): add CvTextInputSkeleton #1542

Closed mateusbandeiraa closed 8 months ago

mateusbandeiraa commented 8 months ago

Contributes to #167

What did you do?

Implemented cv-text-input-skeleton.

Why did you do it?

Because Carbon Vue rocks 😄 . And it was requested on #167.

How have you tested it?

Ran the existing unit tests and added new ones.

Were docs updated if needed?

(I updated the storybook. Is there anything else that needs to be updated?)

davidnixon commented 8 months ago

@mateusbandeiraa I know this is stilla draft but it looks really nice. For me there are only a couple of tweaks to the storybook before this one is ready.

  1. The tag in the template in the storybook is not closed. You can see the error in the console window.
    runtime-core.esm-bundler.js:41 [Vue warn]: Template compilation error: Element is missing end tag.
  2. The story should hide the controls that are not relevant. I think for this one, only the hideLabel is needed. image

Hints

diff --git a/src/components/CvTextInput/CvTextInput.stories.js b/src/components/CvTextInput/CvTextInput.stories.js
index a262e4c7..ea687602 100644
--- a/src/components/CvTextInput/CvTextInput.stories.js
+++ b/src/components/CvTextInput/CvTextInput.stories.js
@@ -244,7 +244,7 @@ Password.parameters = storyParametersObject(
   Password.args
 );

-const templateSkeleton = `<cv-text-input-skeleton v-bind='args'>`;
+const templateSkeleton = `<cv-text-input-skeleton v-bind='args'/>`;
 const TemplateSkeleton = args => {
   return {
     components: { CvTextInputSkeleton },
@@ -254,6 +254,12 @@ const TemplateSkeleton = args => {
 };

 export const Skeleton = TemplateSkeleton.bind({});
+Skeleton.parameters = {
+  controls: {
+    // exclude everything except `hideLabel`
+    exclude: ['helperText', 'invalidMessage', 'etc'],
+  },
+};
 Skeleton.parameters = storyParametersObject(
   Skeleton.parameters,
   templateSkeleton,
mateusbandeiraa commented 8 months ago

Instead of excluding everyting other than hideLabel, I usedinclude: ['hideLabel'].

mateusbandeiraa commented 8 months ago

I'm having a hard time writing the accessibility test. This always pass, not sure if it is working:

it('is accessible', async () => {
  const skeleton = render(CvTextInputSkeleton);
  expect(skeleton.container).toBeAccessible('cv-text-input-skeleton');
})
davidnixon commented 8 months ago

Instead of excluding everything other than hideLabel, I usedinclude: ['hideLabel'].

I din't know "include" was a thing. That would probably help me clean up some of my own tests. Thanks. Nice!

davidnixon commented 8 months ago

I'm having a hard time writing the accessibility test. This always pass, not sure if it is working:

it('is accessible', async () => {
  const skeleton = render(CvTextInputSkeleton);
  expect(skeleton.container).toBeAccessible('cv-text-input-skeleton');
})

This works great for me.

  it('is accessible', async () => {
    const main = document.createElement('main');
    const skeleton = render(CvTextInputSkeleton, {
      container: document.body.appendChild(main),
    });
    await expect(skeleton.container).toBeAccessible('cv-text-input-skeleton');
  }, 10000)

To verify it is working I added a bogus role to the span like:

    <span
      v-if="!hideLabel"
      role="this is bogus"
      :class="[`${carbonPrefix}--label`, `${carbonPrefix}--skeleton`]"
    >

And then the test produces:

Error: Scan: cv-text-input-skeleton

  • Message: The role 'this,is,bogus' defined on the element is not valid per ARIA specification

I removed the bogus role and the test runs cleanly.

mateusbandeiraa commented 8 months ago

@davidnixon I don't plan doing any more changes to this PR if that looks good for you.