-
Notifications
You must be signed in to change notification settings - Fork 156
Port the detailed query page to Vue #2165
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
39a1af6
to
329e45d
Compare
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 don't know the website code too well, and there's a lot of changes, so this is a pretty surface level analysis, but from what I can tell, this is pretty much just one-to-one port so I don't expect new issues here.
For such an internal tool, this seems fine even without changes, so I approved, but I'm not sure about what standard is expected for this repo, so feel free to call me out on this.
little note: Moving sorting client side is nice for UX, but can backfire when there's too much data and you have to start loading incrementally. At my job, we're in the process of moving all these things back to server, it's a bit of a pain. Let's hope we won't hit that limit soon here :D.
Thanks for the review! I did a bunch of cleanups, I underestimated how many of the old "HTML-isms" were kept in the code. |
Let's try. |
We have |
@oli-obk asked me to add some additional query profile data to the table. In order to do that, I moved the page to Vue (with big help from Claude :) ). This also allows us to do client-side sorting of the table, and fix a few sorting related bugs (e.g. sorting by name was somehow buggy before).