carbon-design-system / carbon-components-vue

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

fix(cv-date-picker): improve date picker story #1569

Closed hedint closed 4 months ago

hedint commented 4 months ago

Contributes to #

 What did you do?

Added additional story for the date picker component, which shows how to work with "minDate" and "maxDate" options. Improved argTypes in the date picker story file. Deleted unused prop "invalid" in the date picker component.

 Why did you do it?

To improve component's appearance in the storybook.

How have you tested it?

With local story.

Were docs updated if needed?

davidnixon commented 4 months ago

related to #1566

davidnixon commented 4 months ago

show code

I like this update but I think you should show the code in the story so people can see how you did it.

Maybe something like this:

--- a/src/components/CvDatePicker/CvDatePicker.stories.js
+++ b/src/components/CvDatePicker/CvDatePicker.stories.js
@@ -220,8 +220,31 @@ const TemplateSingleUsingMinMax = args => {
   };
 };

+const codeMinMax = `
+const now = new Date();
+const nextWeek = new Date();
+nextWeek.setDate(now.getDate() + 7);
+const calOptions = ref({
+        minDate: now,
+        maxDate: nextWeek,
+        dateFormat: 'm/d/Y'
+      })
+${templateSingleUsingMinMax}
+`;
+const docMinMax = `
+Example showing how \`calOptions\` can be used to control the min/max date.
+`;
+
 export const SingleUsingMinMax = TemplateSingleUsingMinMax.bind({});
 SingleUsingMinMax.args = initArgs;
+SingleUsingMinMax.parameters = {
+  docs: {
+    source: { code: codeMinMax },
+    description: {
+      story: docMinMax,
+    },
+  },
+};

 /* RANGE USING DATE STORY */

Which shows up on the docs page like this: image

image

properties

Also I think you should move the comments for the properties to the component itself. This would have the added benefit of making the property comments available in IDEs. Here is an example from the copy button in WebStorm: image

See src/components/CvCopyButton/CvCopyButton.vue for reference

examples:

--- a/src/components/CvDatePicker/CvDatePicker.stories.js
+++ b/src/components/CvDatePicker/CvDatePicker.stories.js
@@ -31,11 +31,6 @@ export default {
     },
   },
   argTypes: {
-    dateLabel: { type: String, description: 'Date picker label' },
-    dateEndLabel: {
-      type: String,
-      description: 'Date picker end label (when using kind="range")',
-    },
     invalidMessage: {
       type: String,
       description: 'Date picker text on invalid value',
@@ -220,8 +215,31 @@ const TemplateSingleUsingMinMax = args => {
   };
 };
diff --git a/src/components/CvDatePicker/CvDatePicker.vue b/src/components/CvDatePicker/CvDatePicker.vue
index 6a8af7db..065d3498 100644
--- a/src/components/CvDatePicker/CvDatePicker.vue
+++ b/src/components/CvDatePicker/CvDatePicker.vue
@@ -139,7 +139,9 @@ let calendar;

 const props = defineProps({
   modelValue: { type: [String, Object, Array, Date], default: undefined },
+  /** Date picker label */
   dateLabel: { type: String, default: undefined },
+  /** Date picker end label (when using kind="range") */
   dateEndLabel: { type: String, default: 'End date' },
   disabled: { type: Boolean, default: false },
   invalidMessage: { type: String, default: undefined },
hedint commented 4 months ago

Really nice add for this story. Just a couple of tweaks. See comment.

I've done these tweaks, you can take a look.