-
-
Notifications
You must be signed in to change notification settings - Fork 72
feat: add sorter for DomEvents #195
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
feat: add sorter for DomEvents #195
Conversation
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.
Can we also make the # column title clickable to change direction? That way this PR add's some functionality, in addition to adding the effect dependency. 😇
We can use the chevron-up and chevron-down icons to indicate direction.
|
Yes I can. I'll put the PR on draft until I've done |
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.
Thanks! This is really awesome!
I do think we can simplify it a bit though. In the way I look at this, it doesn't make sense to add sorting to any other column than the id. The event log is meant to see which events a certain action trigger.
Changing the order of events (click -> change -> click to click -> click -> change) , renders the data useless.
So I believe we should only support sorting by id.
If we only sort on ID, we can also simplify this code. We don't need a "sorting" hook. We just need to call buffer.current.reverse() and flip between buffer.current.splice(0, 0, log) and buffer.current.push(log).
What do you think?
What:
Why:
How:
I have also added two dependences
I think we can replace other icons with the one of the library
Checklist: