-
Notifications
You must be signed in to change notification settings - Fork 59
Replace tablesorter lib with custom sorter #1435
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
Conversation
ui/core/components/detailed_results/metrics_table/metrics_table.tsx
Outdated
Show resolved
Hide resolved
ui/core/components/detailed_results/metrics_table/table_sorter.ts
Outdated
Show resolved
Hide resolved
ui/core/components/detailed_results/metrics_table/table_sorter.ts
Outdated
Show resolved
Hide resolved
ui/core/components/detailed_results/metrics_table/table_sorter.ts
Outdated
Show resolved
Hide resolved
ui/core/components/detailed_results/metrics_table/table_sorter.ts
Outdated
Show resolved
Hide resolved
|
|
||
| this.tableElem = this.rootElem.getElementsByClassName('metrics-table')[0] as HTMLTableSectionElement; | ||
| this.bodyElem = this.rootElem.getElementsByClassName('metrics-table-body')[0] as HTMLElement; | ||
| this.tableElem = this.rootElem.querySelector('.metrics-table') as HTMLTableElement; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can pass the HTMLTableElement as a generic: .querySelector<HTMLTableElement>()! and then use ! to tell TS you are sure this is never undefined. Cleaner than typecasting (i tried to fix this in a lot of legacy components)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol used typecasting because it's done everywhere else like that. Should I just regex replace all of them then? :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to get rid of it where I did major refactors 😅😂 feel free to run a find replace regex tbh 👌🫠
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
|
Gonna go ahead and merge this 👍 Thanks @FelixPflaum ! I opened issue in the Classic, Cata, and Wrath repos as well to track porting it |
No description provided.