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

Form component container #6871

Merged
merged 15 commits into from
Mar 19, 2020

Conversation

Jehangir-Wahid
Copy link
Contributor

@Jehangir-Wahid Jehangir-Wahid commented Mar 16, 2020

Adding documentation for the Container ui-component

This pull request (PR) is intended to add the Container ui-component's documentation to the ui-components list.

Affected DevDocs pages

This PR will add a new page to the ui-components. None of the pages will be affected by this PR.

whatsnew
Added a new topic for the Container ui-component.

@devops-devdocs
Copy link
Collaborator

An admin must run tests on this PR before it can be merged.

@ghost
Copy link

ghost commented Mar 16, 2020

Hi @Jehangir-Wahid, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@ghost
Copy link

ghost commented Mar 16, 2020

Hi @Jehangir-Wahid, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@Jehangir-Wahid
Copy link
Contributor Author

I have signed the Adobe CLA twice and it's check still requires signing it. What else am I supposed to do?

Copy link
Contributor

@rogyar rogyar left a comment

Choose a reason for hiding this comment

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

Hi @Jehangir-Wahid. Thank you for the new topic. Please, check my comments below. Also, could I kindly ask you to provide the following information additionally, please?

  • Add "whatsnew" section to the PR description with a sentence/two about the new information provided in this PR. You can check an example here
  • Provide a link to the source code of the Component UI component.

Thank you!

src/guides/v2.3/ui_comp_guide/components/ui-container.md Outdated Show resolved Hide resolved
src/guides/v2.3/ui_comp_guide/components/ui-container.md Outdated Show resolved Hide resolved
@rogyar rogyar added the New Topic A major update published as an entirely new document label Mar 16, 2020
@rogyar
Copy link
Contributor

rogyar commented Mar 16, 2020

You may also add a "Source files" section to the document. Please, check the following PR as an example

https://github.com/magento/devdocs/pull/6872/files#diff-bdabb6f71fb335895e4b4762bd05e102R20

@filmaj
Copy link
Contributor

filmaj commented Mar 16, 2020

@Jehangir-Wahid i can see you signed the CLA twice, but you entered your github username incorrectly.

In the signed document, you entered jehangirwahid as your username. Please enter your username correctly, with the dash included, i.e. Jehangir-Wahid.

Attached a screenshot of what I see in the document database:

Screen Shot 2020-03-16 at 11 47 56 AM

@Jehangir-Wahid
Copy link
Contributor Author

Hi Rogyar,

Thank you for all of the suggestions. I will alter it accordingly shortly.

@Jehangir-Wahid
Copy link
Contributor Author

@filmaj,

Yeah you are absolutely right.
I am going to fill it with the correct username.

Thank you.

@dobooth
Copy link
Contributor

dobooth commented Mar 16, 2020

Asking @serhiyzhovnir to review for consistency with other UI documentation.

@rogyar rogyar added the 2.3.x Magento 2.3 related changes label Mar 17, 2020
Copy link
Contributor

@serhiyzhovnir serhiyzhovnir left a comment

Choose a reason for hiding this comment

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

Hi @Jehangir-Wahid
Thank you for your contribution.
I have the following suggestions for this PR:

Could you, please, check all suggesions and let me know if any questions.

Thank you!

@Jehangir-Wahid
Copy link
Contributor Author

Hi @serhiyzhovnir,

Yeah, for sure.

@Jehangir-Wahid
Copy link
Contributor Author

Hi @serhiyzhovnir,

I have committed some changes in my branch, please review it. Thanks

Also I want to let you know that I didn't find a js file for the container component. Therefore didn't added the component configuration option and also couldn't added any js file path in the Source files section.

Please, guide me if I am missing something and/or if there is a js file for it so that I can add it in the documentation.

Thanks

@dobooth
Copy link
Contributor

dobooth commented Mar 19, 2020

Thanks for the help, gents!

@dobooth
Copy link
Contributor

dobooth commented Mar 19, 2020

running tests

@dobooth dobooth merged commit 04fb132 into magento:master Mar 19, 2020
@ghost
Copy link

ghost commented Mar 19, 2020

Hi @Jehangir-Wahid, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@sidolov sidolov added New Topic A major update published as an entirely new document and removed New Topic A major update published as an entirely new document labels Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.3.x Magento 2.3 related changes New Topic A major update published as an entirely new document
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants