kpmck / cypress-ag-grid

Cypress plugin for interacting with ag grid
30 stars 17 forks source link

Sorting loops if the table does not have aria-sort. #50

Closed strevizo-g closed 1 year ago

strevizo-g commented 1 year ago

Looks like my company did not implement AG Grid properly and sorting will loop because there is no "aria-sort" in the "ag-header-cell" element.

I see other elements with classes that could be used for this validation, maybe add a parameter to use them for sorting? Or throw an error if there is not "aria-sort" attribute.

kpmck commented 1 year ago

Hey there @strevizo-g do you have an example of how your company implemented AG Grid? What are the other attributes that you see in your grid implementation that control the sorting?

strevizo-g commented 1 year ago

This is the element with ag-header-cell. it's missing the aria-sort:

<div class="ag-header-cell ag-focus-managed ag-header-cell-sortable" role="presentation" unselectable="on" tabindex="-1" col-id="name" style="width: 145px; left: 922px;">

I ended up using the "ag-cell-label-container" inside that elements as it contains a class that displays the status: "ag-header-cell-sorted-desc", "ag-header-cell-sorted-asc" or "ag-header-cell-sorted-none"

kpmck commented 1 year ago

@strevizo-g I've verified if I update the sortColumnBy function to use the ag-header-cell-sorted-x class as shown below, it appears to work as expected:

export function sortColumnBy(agGridElement, columnName, sortDirection) {
  if (sortDirection === sort.ascending || sortDirection === sort.descending) {
    return getColumnHeaderElement(agGridElement, columnName)
      .parents(".ag-header-cell .ag-cell-label-container")
      .invoke("attr", "class")
      .then((value) => {
        cy.log(`sort: ${sortDirection}`);
        if(!value.includes(`ag-header-cell-sorted-${sortDirection}`)){
          getColumnHeaderElement(agGridElement, columnName).click().wait(250);
          sortColumnBy(agGridElement, columnName, sortDirection);
        }
      });
  } else {
    throw new Error(
      "sortDirection must be either 'asc' or 'desc'."
    );
  }
}

This may actually be a better implementation to ensure those who do not implement aria attributes (such as your company) can leverage this as well.

I will work more on this later this week to make it backwards compatible and will let you know when a new release is available.

kpmck commented 1 year ago

This bugfix has been released as part of v2.0.4