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

Only the <td> and their values are moved #50

Closed
LouisonDVC opened this issue Aug 25, 2021 · 4 comments
Closed

Only the <td> and their values are moved #50

LouisonDVC opened this issue Aug 25, 2021 · 4 comments

Comments

@LouisonDVC
Copy link

When you sort the table, only the <td> elements are moved, not the <tr>. That could be ok, but the associated ids in the <tr> don't move either, making it difficult if we want to make further treatment on the table (changing data for example)

@LeeWannacott
Copy link
Owner

Just making sure we are on the same page. You mean the <tr classes> are not switching (your problem is shown in the screenshots)?
table11
table22

I'm busy doing a coding bootcamp right now; so not sure when I will get around to fixing it (or possibly having it as an optional table class). There may be situations where the user doesn't want <tr> to switch, although I think the default should be them switching. Thanks for opening the issue. 👍

@LouisonDVC
Copy link
Author

LouisonDVC commented Aug 26, 2021

Yes, that's exactly that! It was "id" in my case, but it's the same problem.
Right now, as I needed it quickly and have a limited use on the library (I just needed a simple sorting application) I just made a home modification in getTableData() to add the id in the array, but it can be improved for all the cases (days of week, files, etc).

getTableData()

if (!isFileSize && !isDayOfWeek && !isDataAttribute) {
    columnData.push(`${tdTextContent}#${i}`);
    columnIndexAndTableRow[`${tdTextContent}#${i}`] = [tr.id, tr.innerHTML];
 }

updateTable()

 else if (!isFileSize) {
    tr.id = columnIndexAndTableRow[columnData[i]][0];
    tr.innerHTML = columnIndexAndTableRow[columnData[i]][1];
}

There are actually cases where we don't know the classes to switch, when the row are striped for example, so it should have a boolean to activate it indeed.

@LeeWannacott
Copy link
Owner

LeeWannacott commented Aug 26, 2021

Sweet, looks like you figured it out!

I would think the way we implement it is by using outerHTML of the <tr> rather than than innerHTML that way you would end up sorting the whole table row and any associated classes or/and id's in the table row.

I was thinking of deprecating days of the week in favor of users using the data-sort attribute; although should probably keep it just in case it breaks for anyone that is currently using it.

Feel free to make a PR if you are interested and have the free time (preferably with some tests written if possible).

Cheers.

@LeeWannacott
Copy link
Owner

Duplicate of: #88 fixed in #89

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants