shipperhq / module-shipper

Base ShipperHQ Repo
Open Software License 3.0
21 stars 21 forks source link

Mixin keeps adding "description" header to shipping table causing columns to misalign #76

Closed squeegy06 closed 4 years ago

squeegy06 commented 4 years ago

The mixin ShipperHQ_Shipper/js/model/shipping-service-mixin has a function appendTooltipColumn which keeps adding a table header despite one already existing, causing the table headers to no longer align with the content.

Preq: SHQ: latest Magento: 2.2.8

Steps: 1) Ensure Onepage Checkout is enabled. 2) Add item to cart 3) Proceed to checkout 4) Enter shipping information 5) Observe, shipping methods are displayed correctly with the column headers aligned properly 6) Change the zip code causing the shipping methods to reload. 7) Observe, after the shipping methods reload the columns are no longer aligned.

Result: The table headers are misaligned.

Expected Result: The table headers are aligned with the content of the table.

It looks like what is happening is Magento only reloads the body of the table using the JS template Magento_Checkout/shipping-address/shipping-method-item while the header remains the same. But since the function only checks the body if it contains "tr td.col-description", the function ends up adding an additional description header every time the checkout recalculates shipping methods.

[EDIT] This may be theme dependent as some table stylings may not care about the number of table headers and table columns being mismatched and still display the content correctly. But if you use your preferred dev tool and inspect the HTML, you will see that a description header keeps getting appended to the table header despite one already existing.

ibraheemnabeelfauzi commented 4 years ago

Hey @squeegy06 can you provide screencast with the store URL, please?

Thank you.

squeegy06 commented 4 years ago

@ibraheemnabeelfauzi you should be able to repeat this issue on any v2.2.8 install of MagentoCE following my steps above. You just may not actually see the table headers get misaligned as some table styles don't care about extra headers, but if you inspect the table's HTML you'll see <th class="col col-description" data-bind="i18n: ''"></th> is repeated in the table's <thead> section each time you change the zip code causing the shipping rates to be recalculated.

Unfortunately I've already made code changes to remedy this issue on our public facing sites, and I don't have means for recording my screen. Even then, it's very obvious when you look at the function "appendTooltipColumn".

In the first line it checks if ($(methodTable).find('tr td.col-description').length === 0)

Since Magento is only replacing the data in the <tbody> of the table via it's template found at "Magento_Checkout/shipping-address/shipping-method-item", the CSS class ".col-description" is not found, and the if() becomes true.

Next, the function immediately appends a heading, without first checking if that heading already exits. And since Magento, again, only replaced the body of the table and not the header, an extra heading gets appended.

var heading = $('<th class="col col-description" data-bind="i18n: \'\'"></th>');
heading.appendTo(methodTable.find('thead tr'));

The result is a table header that looks like this

<thead>
        <tr class="row">
            <th class="col col-method" data-bind="i18n: 'Select Method'">Select Method</th>
            <th class="col col-price" data-bind="i18n: 'Price'">Price</th>
            <th class="col col-method" data-bind="i18n: 'Method Title'">Method Title</th>
            <th class="col col-carrier" data-bind="i18n: 'Carrier Title'">Carrier Title</th>
            <th class="col col-description" data-bind="i18n: ''"></th>
            <th class="col col-description" data-bind="i18n: ''"></th>
        </tr>
</thead>

With as many <th class="col col-description" data-bind="i18n: ''"></th> repeating at the end for each time you recalculated shipping costs without refreshing the page.

ibraheemnabeelfauzi commented 4 years ago

@squeegy06 thanks for that sir. I'll be testing this on 2.3 as 2.2 is EOL: https://magento.com/sites/default/files/magento-software-lifecycle-policy.pdf

wsajason commented 4 years ago

Hi @squeegy06 This has been tested & raised with our development team for further investigation

wsadasmit commented 4 years ago

Hi @squeegy06 , We've released a fix for this issue on the latest version of the extension, 20.37.1. Please upgrade to latest using composer: https://docs.shipperhq.com/installing-magento-2-shipperhq-extension/#Updating_Existing_Installation Thanks again