Skip to content
This repository was archived by the owner on Nov 19, 2024. It is now read-only.

Add documentation for the DynamicRowsRecord UI component #5634

Merged
merged 4 commits into from
Oct 21, 2019
Merged

Add documentation for the DynamicRowsRecord UI component #5634

merged 4 commits into from
Oct 21, 2019

Conversation

serhiyzhovnir
Copy link
Contributor

@serhiyzhovnir serhiyzhovnir commented Oct 9, 2019 •
edited by meker12
Loading

Purpose of this pull request

This pull request (PR) adds the documentation for the DynamicRowsRecord UI component.

Affected DevDocs pages

Links to Magento source code

whatsnew
Added the documentation for the DynamicRowsRecord UI component to the UI Components Guide.

@devops-devdocs
Copy link
Collaborator

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

1 similar comment
@devops-devdocs
Copy link
Collaborator

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

@rogyar rogyar self-assigned this Oct 9, 2019
rogyar
rogyar suggested changes Oct 9, 2019
View reviewed changes
guides/v2.2/ui_comp_guide/components/ui-dynamicrows-record.md Outdated
| Option | Description | Type | Default |
| --- | --- | --- | --- |
| `disabled` | Initial component's state. When set to `true`, users can't take action on the element. | Boolean | `true` |
| `headerLabel` | The label of the record. It is used as a record `label` in case if the `label` option is not initialized. | String | `''` |
Copy link
Contributor

Choose a reason for hiding this comment

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

Does that mean that headerLabel value is just a fallback for label value? Are you sure that's the only purpose of this field?
Maybe it is, I'm just asking because having this flow looks a little bit weird.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @rogyar
Yes, it looks strange but it's how it works.
There are two places in the Magento where this option is used
https://github.com/magento/magento2/blob/2.2/app/code/Magento/Ui/view/base/web/js/dynamic-rows/record.js#L85 - here the headerLabel option is replacing the label option if the label option is not configured.
https://github.com/magento/magento2/blob/2.2/app/code/Magento/Ui/view/base/web/js/dynamic-rows/record.js#L209 - here the headerLabel option returns as a result of getLabel() method in case if the label argument is not a string (or label option is not specified) or if the label option is not configured.
@rogyar, please, let me know if I need to improve something here.
Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the clarification

@rogyar rogyar added 2.2.x 2.3.x Magento 2.3 related changes New Topic A major update published as an entirely new document labels Oct 9, 2019
@keharper keharper added the Waiting for Response Waiting for response from internal/external parties label Oct 15, 2019
@rogyar rogyar removed the Waiting for Response Waiting for response from internal/external parties label Oct 16, 2019
@keharper keharper self-assigned this Oct 16, 2019
keharper
keharper suggested changes Oct 16, 2019
View reviewed changes
Copy link
Contributor

@keharper keharper left a comment

Choose a reason for hiding this comment

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

A few small editorial changes.

guides/v2.2/ui_comp_guide/components/ui-dynamicrows-record.md Outdated
contributor_link: https://www.atwix.com/
---

The DynamicRowsRecord component is a container of record fields. The DynamicRowsRecord should be used as a child of [DynamicRows]({{ page.baseurl }}/ui_comp_guide/components/ui-dynamicrows.html) component.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The DynamicRowsRecord component is a container of record fields. The DynamicRowsRecord should be used as a child of [DynamicRows]({{ page.baseurl }}/ui_comp_guide/components/ui-dynamicrows.html) component.
The DynamicRowsRecord component is a container of record fields. The DynamicRowsRecord should be used as a child of the [DynamicRows]({{ page.baseurl }}/ui_comp_guide/components/ui-dynamicrows.html) component.

guides/v2.2/ui_comp_guide/components/ui-dynamicrows-record.md Outdated

| Option | Description | Type | Default |
| --- | --- | --- | --- |
| `disabled` | Initial component's state. When set to `true`, users can't take action on the element. | Boolean | `true` |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| `disabled` | Initial component's state. When set to `true`, users can't take action on the element. | Boolean | `true` |
| `disabled` | The initial state of the component. When set to `true`, users cannot take action on the element. | Boolean | `true` |

guides/v2.2/ui_comp_guide/components/ui-dynamicrows-record.md Outdated
| Option | Description | Type | Default |
| --- | --- | --- | --- |
| `disabled` | Initial component's state. When set to `true`, users can't take action on the element. | Boolean | `true` |
| `headerLabel` | The label of the record. It is used as a record `label` in case if the `label` option is not initialized. | String | `''` |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| `headerLabel` | The label of the record. It is used as a record `label` in case if the `label` option is not initialized. | String | `''` |
| `headerLabel` | The label of the record. It is used as a record `label` when the `label` option is not initialized. | String | `''` |

@keharper
Copy link
Contributor

Also, if you haven't done so already please optimize the image by running rake check:image_optim common/images/ui_comps/dynamicrows-record-result.png from your devdocs directory.

@keharper
Copy link
Contributor

@serhiyzhovnir Making sure you saw my review.

@serhiyzhovnir
Copy link
Contributor Author

Hi @keharper
I will fix the issues from the review soon.
Thanks!

@serhiyzhovnir
Copy link
Contributor Author

Hi @keharper
I've adjusted the PR.
The common/images/ui_comps/dynamicrows-record-result.png image is already optimized via https://imagecompressor.com/.
Could you, please, check the PR?
Thank you!

@keharper
Copy link
Contributor

running tests

@keharper keharper merged commit fa55887 into magento:master Oct 21, 2019
@ghost
Copy link

ghost commented Oct 21, 2019

Hi @serhiyzhovnir, 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.

@keharper
Copy link
Contributor

Thanks @serhiyzhovnir

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rogyar rogyar rogyar approved these changes

@keharper keharper keharper approved these changes

Assignees

@rogyar rogyar

@keharper keharper

Labels
2.2.x 2.3.x Magento 2.3 related changes New Topic A major update published as an entirely new document Partner: Atwix partners-contribution PR created by Magento partner
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@serhiyzhovnir @devops-devdocs @keharper @rogyar @magento-engcom-team