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

Fixed a js bug where ui_component labels have the wrong sort order. #11846

Merged

Conversation

deiserh
Copy link
Contributor

@deiserh deiserh commented Oct 29, 2017

Description

If you extend an dynamic-row element in a ui_component and add a sort order attribute with a amount between the others, the Fields are in the correct order but the label is always the last one.

Manual testing scenarios

  1. Extend an dynamic-row in a ui_component with a sort order so your element isn't the last one.
    Result: The label are in the correct sort order (your Field Label isn't the last one).

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Oct 29, 2017

CLA assistant check
All committers have signed the CLA.

* additional added field labels in ui_components haven't the right
* sort order.
*/
this.labels.sort(this._compare);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you, please, move push and sort calls outside of loop for performance improvement. Also, I think it's better to use _.sortBy() function and remove custom compare function. An example:

initHeader: function () {
    var labels = [],
        data;

    if (!this.labels().length) {
        _.each(this.childTemplate.children, function (cell) {
            data = this.createHeaderTemplate(cell.config);
            cell.config.labelVisible = false;
            _.extend(data, {
                label: cell.config.label,
                name: cell.name,
                required: !!cell.config.validation,
                columnsHeaderClasses: cell.config.columnsHeaderClasses,
                sortOrder: cell.config.sortOrder
            });

            labels.push(data);
        }, this);
        this.labels(_.sortBy(labels, 'sortOrder'));
    }
}

@@ -548,6 +548,13 @@ define([
});
Copy link
Contributor

Choose a reason for hiding this comment

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

It's would be nice if you will mark dynamic-rows-tier-price.js as deprecated, because it was created by mistake for fix same issue, but just for tier price functionality.

@omiroshnichenko
Copy link
Contributor

Hi, @deiserh. I have made update for your code. Thank you for contribution.

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.

6 participants