-
Notifications
You must be signed in to change notification settings - Fork 26
Make project.description optional #5
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
Conversation
5e6fa35 to
8d5e782
Compare
sylwiavargas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this totally makes sense.
8d5e782 to
fb61bc9
Compare
fb61bc9 to
4615fe5
Compare
| export interface Project { | ||
| title: string; | ||
| description: string; | ||
| description?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🌟 Main type change
| form.appendChild(createHiddenInput('project[template]', project.template)); | ||
| addInput('project[title]', title); | ||
| if (typeof description === 'string' && description.length > 0) { | ||
| addInput('project[description]', description); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did a bit of refactoring of the createProjectForm function, but the main functional change is here: conditionally creating an input to send the description in the POST request.
| <head><title></title></head> | ||
| <body> | ||
| <form method=\\"POST\\" style=\\"display:none!important;\\" action=\\"https://stackblitz.com/run?embed=1\\" id=\\"sb_run\\"><input type=\\"hidden\\" name=\\"project[title]\\" value=\\"Test Project\\"><input type=\\"hidden\\" name=\\"project[description]\\" value=\\"This is a test\\"><input type=\\"hidden\\" name=\\"project[template]\\" value=\\"javascript\\"><input type=\\"hidden\\" name=\\"project[files][package.json]\\" value=\\"{ | ||
| <form method=\\"POST\\" style=\\"display:none!important;\\" action=\\"https://stackblitz.com/run?embed=1\\" id=\\"sb_run\\"><input type=\\"hidden\\" name=\\"project[title]\\" value=\\"Test Project\\"><input type=\\"hidden\\" name=\\"project[template]\\" value=\\"javascript\\"><input type=\\"hidden\\" name=\\"project[files][package.json]\\" value=\\"{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those snapshots are a bit hard to read, especially in GitHub diffs, but they're correct as far as I can tell.
| // Check input values | ||
| expect(value('project[title]')).toBe('My Test Project'); | ||
| expect(value('project[description]')).toBe(project.description); | ||
| expect(value('project[description]')).toBe(undefined); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the description from the default test project.
When using the SDK's
embedProjectandopenProjectmethods, which expect a project definition object, our TypeScript types require thedescriptionto be provided.Turns out that the StackBlitz backend doesn't actually require a description.
In my own tests, having to come up with a description for a simple example, test or reproduction is often a chore. I've definitely used
description: ''before as a workaround. I think we can get out of our users's way and make the description field optional.What do you think?