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

966 add a placeholders to create new controller form #962

Conversation

SilinMykola
Copy link

@SilinMykola SilinMykola commented Feb 7, 2022

Description (*)
I added the placeholders to the 'Create new Controller' form.
actionPath

Fixed Issues (if relevant)

  1. Fixes New Magento2 Controller. Add a placeholder. #960

Questions or comments

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with integration/functional tests (if applicable)
  • All automated tests passed successfully (all builds are green)

@SilinMykola
Copy link
Author

@eduard13 hi! Could you please review the PR and tell me what you think about the placeholders on this form. Will the input fields be understandable with this kind of placeholders?
CC: @lfolco hello! What do you think about these placeholders? Would that be helpful?

@lfolco
Copy link

lfolco commented Feb 7, 2022

Still not sure exactly what is being asked for in these: is Controller Name supposed to be something along the lines of catalog/product/view?

@bohdan-harniuk
Copy link
Collaborator

Hello, @lfolco!

Yes, controller name for the provided path catalog/product/view is a Product.
How do you think it would be better to describe it in the placeholder?

Regards,

@lfolco
Copy link

lfolco commented Feb 7, 2022

After testing it locally, I think the labels should be Action Name and Action Path. Placeholder can be Class name for Action and Folder name for Action.

Unless the plugin has been changed, it doesn't look like it's doing anything along the lines of catalog/product/view.

Also, it doesn't create a proper namespace. When I fill it in like this:

image

I get a class with a namespace like this:

image

What exactly is Parent Directory supposed to be? You can't change it from what I can see.

@bohdan-harniuk
Copy link
Collaborator

@lfolco, you have outdated plugin version. Please, update it and try it again. Everything should works fine with namespaces.
Parent directory is a constant value. It doesn't matter because all controllers always are in the Controller folder.

Regards,

@SilinMykola SilinMykola force-pushed the 960-add-a-placeholder-create-new-controller branch from 20ec0ec to 24490f3 Compare February 9, 2022 13:08
@SilinMykola
Copy link
Author

@lfolco Hello! I think you are right. I changed labels and placeholders. Our goal is to make controller creation more understandable for everyone. Please, check new variants. Thanks!
actionPath
CC: @bohdan-harniuk Hi! What do you think about these changes? Thanks!

@bohdan-harniuk
Copy link
Collaborator

Hello, @SilinMykola, @lfolco!

I think you made great improvements here!

Thank you!

Copy link
Collaborator

@bohdan-harniuk bohdan-harniuk left a comment

Choose a reason for hiding this comment

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

Great work!

@SilinMykola SilinMykola deleted the 960-add-a-placeholder-create-new-controller branch February 9, 2022 13:44
@lfolco
Copy link

lfolco commented Feb 9, 2022

This looks great, thank you!

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

Successfully merging this pull request may close these issues.

New Magento2 Controller. Add a placeholder.
3 participants