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

Feature/added cli option checkboxes #18

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

radoslawkoziol
Copy link

@radoslawkoziol radoslawkoziol commented Dec 26, 2024

Enhanced CLI Modal and Plugin Compatibility Updates

Changes

CLI Modal Improvements

image

  • Added dynamic "generation options" checkboxes to the CLI modal
  • This enhancement simplifies parameter input for users

UI Improvements

image

  • Added a separator in the menu between "Directory" and "Nest Controller" entries
  • Improves menu readability and visual organization

Plugin Compatibility

  • Modified plugin compatibility settings to support future IDE versions
  • Changed pluginUntilBuild to 253.* as a working solution
    • Note: According to documentation, pluginUntilBuild = or pluginUntilBuild = * should work, but testing showed otherwise
    • The implemented solution (253.*) has been verified to work correctly

Code Quality

  • Performed code refactoring to improve maintainability
  • Removed .idea/gradle.xml file from repository to keep it clean

Summary

These changes focus on improving user experience with better CLI parameter handling, clearer menu organization, and ensuring the plugin works with future IDE versions.

@dinbtechit
Copy link
Owner

Hi @radoslawkoziol ,

Thank you for submitting a PR. I really appreciate. However, I have some concerns with the UX here. I understand UX is subjected to person to person. The Checkbox style UX adds more confusion than helping with the ease of use. As it now has 2 input points, one via the textfield and the checkbox. Its also making difficult to address the error validation scenarios. Especially the you still have to use the the parameter textfield to input the filename. There is an autocomplete (Ctrl + space or Cmd + Space) that lists all the available options.

I would definitely up discussing a better UX approach if you want to start a discussion over that and get community feedback before implementing them. Please let me know I can certainly start one.

Additionally, the pluginUntilBuild is set to 253.* which is an invalid build number intellij hasn't released an IDE with that version yet this will cause error in the plugin verification stage of CI/CD. I am working on a more of an automated approach to support all future builds.

Now that the negatives aside. I do like the idea of adding separator in the context menu. It does provide a neat separation from rest of the menu items. Cheers for that.

Hopefully my above comments make sense. If you agree with the can update the PR accordingly? 🙌

Copy link
Owner

@dinbtechit dinbtechit left a comment

Choose a reason for hiding this comment

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

Please see above comments.

gradle.properties Outdated Show resolved Hide resolved

# IntelliJ Platform Properties -> https://plugins.jetbrains.com/docs/intellij/tools-gradle-intellij-plugin.html#configuration-intellij-extension
platformType = IU
platformVersion = 2022.3.3

# Plugin Dependencies -> https://plugins.jetbrains.com/docs/intellij/plugin-dependencies.html
# Example: platformPlugins = com.intellij.java, com.jetbrains.php:203.4449.22
platformPlugins = PsiViewer:2022.3, com.github.dinbtechit.vscodetheme:1.10.2
platformPlugins = PsiViewer:2022.3
Copy link
Owner

Choose a reason for hiding this comment

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

Any specific reason for removing this? This Plugin will not be bundled with the plugin. I added this to do some testing for my VSCode plugin.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I know it doesn't affect the build in any way, but I didn't have it installed, causing it to throw me errors in the console. I decided that this plugin was not necessary, so, to keep things clean, I removed it from here. I can, of course, restore it ;)

Ideally, I would like to throw some of this stuff (especially platformType and platformVersion) into some local configuration - env or something like that. I personally tested these things on a newer version of WebStorm

@radoslawkoziol
Copy link
Author

Thank you for submitting a PR. I really appreciate. However, I have some concerns with the UX here. I understand UX is subjected to person to person. The Checkbox style UX adds more confusion than helping with the ease of use. As it now has 2 input points, one via the textfield and the checkbox. Its also making difficult to address the error validation scenarios. Especially the you still have to use the the parameter textfield to input the filename. There is an autocomplete (Ctrl + space or Cmd + Space) that lists all the available options.

I was also considering this, but I decided that in this form it works quite efficiently. Maybe I didn’t clarify it in the comment, but the text field and checkboxes are linked - checking/unchecking a checkbox affects the text field, and at the same time, removing from the text field unchecks the checkbox.

I’m aware of the autocomplete feature, but typing it every time was tiresome for me - and in cases like services, I mostly use the --flat option. Alternatively, we could make it so that only the checkboxes remain, without displaying the options as text in the input

Additionally, I think an improvement could be to make the selected flags persist based on what was selected the last time - depending on the type of what is being generated. I could add this functionality, assuming it’s possible - which it most likely is.

I would definitely up discussing a better UX approach if you want to start a discussion over that and get community feedback before implementing them. Please let me know I can certainly start one.

Sure 😉

Additionally, the pluginUntilBuild is set to 253.* which is an invalid build number intellij hasn't released an IDE with that version yet this will cause error in the plugin verification stage of CI/CD. I am working on a more of an automated approach to support all future builds.

Ok, of course, I can revert the version, but are you sure it won’t work? I ran the verifyPlugin task, and the check passed successfully.

Now that the negatives aside. I do like the idea of adding separator in the context menu. It does provide a neat separation from rest of the menu items. Cheers for that.

Hopefully my above comments make sense. If you agree with the can update the PR accordingly? 🙌

Sure, your comments absolutely make sense. Let’s decide what to do with the improvement (checkboxes) I proposed, and I’ll make the necessary adjustments :)

I use your plugin almost every day when working with NestJS, and I really like it (great job!). As you’ve probably noticed, I’ve commented on other issues a few times before. So I decided to contribute something from myself as well, and in the process see what the development of plugins looks like since I have never dealt with it before.

@dinbtechit
Copy link
Owner

@radoslawkoziol

Sure, your comments absolutely make sense. Let’s decide what to do with the improvement (checkboxes) I proposed, and I’ll make the necessary adjustments :)

I have created a discussion here to discuss the UX improvement - #19. Lets document the pain points of the existing UI.

I use your plugin almost every day when working with NestJS, and I really like it (great job!). As you’ve probably noticed, I’ve commented on other issues a few times before. So I decided to contribute something from myself as well, and in the process see what the development of plugins looks like since I have never dealt with it before.

Thanks mate. I appreciate the extra helping hand. People like you make the open source community awesome 🙌. There are lots of things we can be working on for this plugin - Please take a look at some of the open issues and see if you can tackle any of the open issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants