Add Story & Storybook Documentation for Dialog Component #507

guergana · 2021-11-30T12:51:25Z

github-actions · 2021-11-30T12:51:56Z

https://85c5e2eac504fed25447865ef2d0e96071f9d314--wmde-wikit.netlify.app

guergana · 2021-11-30T15:51:24Z

vue-components/stories/Dialog.stories.ts

+					ref="simple"
+					:actions="actions"
+					style="transform: translate(50%, 50%)"
+					@action="(_, dialog) => dialog.hide()"


I took this from the MMF. should this be the generic action?

I am not sure if I understand the question? If you mean to say should the dialog just close on any button click then yes, it should

right now there is only one action, yes, so both buttons close the window. is this desired = should i just leave it like this? :)

I think so, yes

vue-components/stories/Dialog.stories.ts

guergana · 2021-11-30T16:28:51Z

vue-components/src/components/Dialog.vue

 			this.$emit( 'dismissed' );
 			this.hide();
 		},
 		_dispatch( namespace: string ): void {
+			/**


Prob needs a better explanation. I don't know how to write it because i don't understand how it works 100%.

itamargiv

Looks good altogether, there are still missing descriptions for each of the props in the controls and docs tab of the story. We can add those as docbloc comments above the definition of each property in the component's vue file.

Additionally, we have an issue with the overly not showing up. Probably due to the changes @Silvan-WMDE and I made by converting the top: 0, left: 0, right: 0, bottom: 0 css positions of the overlay in the vue file to the inset shorthand (which might not work as expected).

vue-components/src/components/Dialog.vue

itamargiv · 2021-12-01T09:55:23Z

vue-components/src/components/Dialog.vue

 			this.$emit( 'dismissed' );
 			this.hide();
 		},
 		_dispatch( namespace: string ): void {
+			/**


vue-components/src/components/Dialog.vue

vue-components/stories/Dialog.stories.ts

sai-san · 2021-12-01T11:31:51Z

vue-components/stories/Dialog.stories.ts

+		}
+	],
+	visible: true,
+	dialogText: `Lorem ipsum dolor sit amet, consectetur adipiscing elit. 


Heads-up that this should be replaced by the copy linked from the ticket. See copy in Figma.

but without the line breaks, right? since the <br/> neither the \n are rendered

Co-authored-by: Itamar Givon <itamar.givon.dev@gmail.com>

Co-authored-by: sai <sarai.sanchez@wikimedia.de>

Co-authored-by: Itamar Givon <itamar.givon.dev@gmail.com>

Co-authored-by: sai <sarai.sanchez@wikimedia.de>

Co-authored-by: Itamar Givon <itamar.givon.dev@gmail.com>

vue-components/stories/Dialog.stories.ts

sai-san · 2021-12-01T16:30:59Z

A couple of issues:

The overllay is still not displayed behind the dialog (it's positioned to the bottom right)

Is there a way to increase the size of the canvas in the Docs page so the Dialog is not displayed on top of the props table? (see screenshot above too)(This was done with the Popover component)

A request for a change that I could apply myself in a separate PR if that sounds better:

The size of the icon in the close button should be large. (This would basically require changing line 41 in the Dialog.vue file to )

Thank you! C'mon we're almost there 🙌

itamargiv · 2021-12-01T16:16:45Z

vue-components/stories/Dialog.stories.ts

+					ref="simple"
+					:actions="actions"
+					style="transform: translate(50%, 50%)"
+					@action="(_, dialog) => dialog.hide()"


I think so, yes

itamargiv · 2021-12-01T16:18:07Z

vue-components/stories/Dialog.stories.ts

+		}
+	],
+	visible: true,
+	dialogText: `Complex dialogs can display extensive information and contain all kinds of interactive elements


I would maybe change the name of this control to content, to make it a little more storybook-user friendly.

ah yes, good point

Co-authored-by: sai <sarai.sanchez@wikimedia.de>

guergana · 2021-12-01T16:51:45Z

A couple of issues:

1. The **overllay is still not displayed behind** the dialog (it's positioned to the bottom right)

2. Is there a way to **increase the size of the canvas in the Docs page** so the Dialog is not displayed on top of the props table? (see screenshot above too)(This was done with the [Popover component](https://wmde.github.io/wikit/?path=/docs/vue_popover--basic))

A request for a change that I could apply myself in a separate PR if that sounds better:

3. The **size of the icon in the close button** should be `large`. (This would basically require changing line 41 in the Dialog.vue file to )

Thank you! C'mon we're almost there raised_hands

Yes, the canvas can be resized, no problem. The button I would suggest yes making a separate PR for that since that's not part of the story itself, but it's a fix to the the dialog component, right?

itamargiv · 2021-12-01T17:10:16Z

vue-components/stories/Dialog.stories.ts

@@ -12,7 +12,7 @@ export function complex( args: { title: string, actions: string, dismissButton:
 		components: { Button, Dialog },
 		props: Object.keys( args ),
 		template: `
-			<div>
+			<div style="height: 70vh;">


what does this fix?

It increases the height of the canvas section in the Docs page, so the Dialog does not overlap with the props table (see my comment above).

oh, yeah, just saw it now

itamargiv

Approving for now, as the overlay will be fixed in another PR.

itamargiv

Approving again with the same comment as before

Add scaffold for Simple and Complex Dialog stories

f3da1ab

guergana changed the title ~~Add Dialog Story~~ Add Story & Storybook Documentation for Dialog Component Nov 30, 2021

guergana marked this pull request as draft November 30, 2021 12:52

guergana commented Nov 30, 2021

View reviewed changes

Add controls to complex Dialog

9f94564

guergana force-pushed the add-story-dialog branch from 42ddb30 to 9f94564 Compare November 30, 2021 16:06

guergana commented Nov 30, 2021

View reviewed changes

vue-components/stories/Dialog.stories.ts Outdated Show resolved Hide resolved

guergana marked this pull request as ready for review November 30, 2021 16:10

Add comments for events

800c0c2

guergana commented Nov 30, 2021

View reviewed changes

sai-san self-requested a review November 30, 2021 16:56

itamargiv requested changes Dec 1, 2021

View reviewed changes

sai-san reviewed Dec 1, 2021

View reviewed changes

vue-components/stories/Dialog.stories.ts Outdated Show resolved Hide resolved

sai-san reviewed Dec 1, 2021

View reviewed changes

vue-components/stories/Dialog.stories.ts Outdated Show resolved Hide resolved

sai-san reviewed Dec 1, 2021

View reviewed changes

vue-components/stories/Dialog.stories.ts Outdated Show resolved Hide resolved

sai-san reviewed Dec 1, 2021

View reviewed changes

vue-components/stories/Dialog.stories.ts Outdated Show resolved Hide resolved

sai-san reviewed Dec 1, 2021

View reviewed changes

guergana and others added 10 commits December 1, 2021 18:23

Update vue-components/src/components/Dialog.vue

4ac47f0

Co-authored-by: Itamar Givon <itamar.givon.dev@gmail.com>

Update vue-components/src/components/Dialog.vue

6dad55b

Co-authored-by: Itamar Givon <itamar.givon.dev@gmail.com>

Update vue-components/stories/Dialog.stories.ts

dd613cb

Co-authored-by: sai <sarai.sanchez@wikimedia.de>

Update vue-components/stories/Dialog.stories.ts

5463b61

Co-authored-by: sai <sarai.sanchez@wikimedia.de>

Update vue-components/src/components/Dialog.vue

a24bff9

Co-authored-by: Itamar Givon <itamar.givon.dev@gmail.com>

Update vue-components/stories/Dialog.stories.ts

54e4b1c

Co-authored-by: sai <sarai.sanchez@wikimedia.de>

Update vue-components/stories/Dialog.stories.ts

a9a085d

Co-authored-by: sai <sarai.sanchez@wikimedia.de>

Update vue-components/stories/Dialog.stories.ts

3878c1f

Co-authored-by: Itamar Givon <itamar.givon.dev@gmail.com>

Update dialog text

e48308b

Add descriptions for props

fc21036

sai-san reviewed Dec 1, 2021

View reviewed changes

vue-components/stories/Dialog.stories.ts Outdated Show resolved Hide resolved

itamargiv reviewed Dec 1, 2021

View reviewed changes

guergana and others added 2 commits December 1, 2021 19:37

Update vue-components/stories/Dialog.stories.ts

f716191

Co-authored-by: sai <sarai.sanchez@wikimedia.de>

Changed control name from "dialogText" to "content"

d79ae00

Set canvas height to 70vh

52c1e9e

itamargiv reviewed Dec 1, 2021

View reviewed changes

itamargiv requested review from itamargiv and removed request for sai-san December 1, 2021 17:22

itamargiv previously approved these changes Dec 1, 2021

View reviewed changes

Remove trailing spaces in comments

e591371

guergana dismissed itamargiv’s stale review via e591371 December 2, 2021 15:34

itamargiv approved these changes Dec 2, 2021

View reviewed changes

guergana merged commit fb437b0 into master Dec 2, 2021

guergana deleted the add-story-dialog branch December 2, 2021 16:14

sai-san mentioned this pull request Dec 3, 2021

Dialog fixes #508

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Story & Storybook Documentation for Dialog Component #507

Add Story & Storybook Documentation for Dialog Component #507

guergana commented Nov 30, 2021 •

edited

github-actions bot commented Nov 30, 2021

guergana Nov 30, 2021

itamargiv Dec 1, 2021

guergana Dec 1, 2021

itamargiv Dec 1, 2021

guergana Nov 30, 2021 •

edited

itamargiv Dec 1, 2021

itamargiv left a comment

itamargiv Dec 1, 2021

sai-san Dec 1, 2021

guergana Dec 1, 2021

sai-san commented Dec 1, 2021 •

edited

itamargiv Dec 1, 2021

itamargiv Dec 1, 2021

guergana Dec 1, 2021

guergana commented Dec 1, 2021 •

edited

itamargiv Dec 1, 2021

sai-san Dec 1, 2021

itamargiv Dec 1, 2021

itamargiv left a comment

itamargiv left a comment

Add Story & Storybook Documentation for Dialog Component #507

Add Story & Storybook Documentation for Dialog Component #507

Conversation

guergana commented Nov 30, 2021 • edited

github-actions bot commented Nov 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guergana Nov 30, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itamargiv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sai-san commented Dec 1, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guergana commented Dec 1, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itamargiv left a comment

Choose a reason for hiding this comment

itamargiv left a comment

Choose a reason for hiding this comment

guergana commented Nov 30, 2021 •

edited

guergana Nov 30, 2021 •

edited

sai-san commented Dec 1, 2021 •

edited

guergana commented Dec 1, 2021 •

edited