Skip to content
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

chore(design): implement design system for generate command #1302

Merged
merged 14 commits into from
Apr 20, 2024

Conversation

Shurtu-gal
Copy link
Collaborator

@Shurtu-gal Shurtu-gal commented Mar 27, 2024

Description

  • An initial prototype of how to bring the envisioned design to reality.
  • Implement design system in generate command.

Related issue(s)
See #1214

@Shurtu-gal
Copy link
Collaborator Author

@Mayaleeeee could you please confirm if the following is according to your design or needs something to be changed.

image

cc: @Amzani

Copy link
Collaborator

@Amzani Amzani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice PR @Shurtu-gal 🚀 , I added few small suggestion.

src/commands/generate/fromTemplate.ts Outdated Show resolved Hide resolved
src/commands/generate/fromTemplate.ts Show resolved Hide resolved
src/commands/generate/fromTemplate.ts Show resolved Hide resolved
@Shurtu-gal
Copy link
Collaborator Author

@Amzani I have applied the suggestions.

@Shurtu-gal Shurtu-gal marked this pull request as ready for review March 28, 2024 06:45
@Mayaleeeee
Copy link
Member

Mayaleeeee commented Mar 28, 2024

@Mayaleeeee could you please confirm if the following is according to your design or needs something to be changed.

image

cc: @Amzani

Thanks for tagging @Shurtu-gal.

Firstly, I'd like to clarify: is the design above intended to display two variations? I notice 'AsyncAPI Generate Models' appearing twice with nearly identical content.

As per my design, when users input 'AsyncAPI Generate Models,' the prompt should be 'Name of the programming language?' instead of what's shown in the image.

@Shurtu-gal
Copy link
Collaborator Author

Firstly, I'd like to clarify: is the design above intended to display two variations? I notice 'AsyncAPI Generate Models' appearing twice with nearly identical content.

As per my design, when users input 'AsyncAPI Generate Models,' the prompt should be 'Name of the programming language?' instead of what's shown in the image.

No it is the same variant, just that initially it will be a select and then the above one when a language has been selected.

@Mayaleeeee
Copy link
Member

Firstly, I'd like to clarify: is the design above intended to display two variations? I notice 'AsyncAPI Generate Models' appearing twice with nearly identical content.

As per my design, when users input 'AsyncAPI Generate Models,' the prompt should be 'Name of the programming language?' instead of what's shown in the image.

No it is the same variant, just that initially it will be a select and then the above one when a language has been selected.

Understood, thanks for clarifying that.

However, you didn't address the second part, which is whether the image you provided aligns with my design.

@Shurtu-gal
Copy link
Collaborator Author

However, you didn't address the second part, which is whether the image you provided aligns with my design.

It can be changed easily; however name of the programming language expects a string to be given as an input, that's why I used select.

@fmvilas
Copy link
Member

fmvilas commented Mar 28, 2024

I also believe that "Select the language..." is better than "Name of the programming language" because the first one is telling you what action to do, i.e., "select". Just my two cents.

@Shurtu-gal Shurtu-gal changed the title chore: initial prototype for how the design system would look like chore(design): implement design system for generate command Apr 1, 2024
Copy link
Collaborator

@Amzani Amzani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Shurtu-gal
Copy link
Collaborator Author

Should I add a flag in all commands to not be interactive if passed? Due to this many tests are failing.

cc: @fmvilas @Amzani

@Shurtu-gal
Copy link
Collaborator Author

Should I add a flag in all commands to not be interactive if passed? Due to this many tests are failing.

cc: @fmvilas @Amzani

cc: @Souvikns

@Souvikns
Copy link
Member

Should I add a flag in all commands to not be interactive if passed? Due to this many tests are failing.

Yeah, I think you should do that.

@Shurtu-gal
Copy link
Collaborator Author

Should I add a flag in all commands to not be interactive if passed? Due to this many tests are failing.

Yeah, I think you should do that.

Great !!

@Shurtu-gal
Copy link
Collaborator Author

@Amzani @Souvikns added the --no-interactive flag. Tests are working great. Would appreciate it if you could review this.

@Souvikns
Copy link
Member

Souvikns commented Apr 19, 2024

@Shurtu-gal you have some conflicts, can you resolve them before I approve?

@Shurtu-gal
Copy link
Collaborator Author

@Shurtu-gal you have some conflicts, can you resolve them before I approve?

@Souvikns done!

Copy link

sonarcloud bot commented Apr 20, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Member

@Souvikns Souvikns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@Shurtu-gal
Copy link
Collaborator Author

/rtm

@asyncapi-bot asyncapi-bot merged commit 6ba4b47 into asyncapi:master Apr 20, 2024
11 checks passed
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 1.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@aeworxet
Copy link
Contributor

@asyncapi/bounty_team

@asyncapi-bot asyncapi-bot added the bounty AsyncAPI Bounty program related label label Apr 29, 2024
@aeworxet aeworxet mentioned this pull request May 7, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty AsyncAPI Bounty program related label ready-to-merge released
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

Implement new UI/UX improvements in generate command
7 participants