Audit log - #2860
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Can't repro locally, but this happened twice in CI so it's probably not a fluke. 
|
fakemonster
left a comment
fakemonster
left a comment
There was a problem hiding this comment.
couple visual issues that don't necessarily fall on a single line:
- the non-datepicker popovers (docs info, refresh interval picker, range picker) fall behind the header because it's sticky. you can add a higher z-index to them and get away with it but it's pretty screwy. better to lower the sticky header to z-10, and then add an
isolateclass to the entire table to put it in a different stacking context (so it doesn't cover the header, which i assume is the original purpose of the z-20) - this isn't exactly using a side modal, but z-index-wise, it needs some sort of... sidemodality. right now tooltips render behind the detail view
| className={cn( | ||
| state.isOpen && 'z-10 ring-2', | ||
| 'text-sans-md border-default hover:border-raise bg-default relative flex h-11 items-center rounded-l-md rounded-r-md border focus-within:ring-2 focus:z-10', | ||
| 'text-sans-md border-default hover:border-raise bg-default relative flex h-10 items-center rounded-l-md rounded-r-md border focus-within:ring-2 focus:z-10', |
There was a problem hiding this comment.
i'm starting to suspect there's a common visual component between ListBox and these pickers that could perhaps even do something a little more graceful with rounding. will think about this for a followup
There was a problem hiding this comment.
That'd be great, we're also repeating some of this in a few places that's more brittle than a shared class or component.
| > | ||
| <div className="bg-raise border-secondary flex h-10 items-center justify-between border-b px-2"> | ||
| <div className="flex items-center"> | ||
| <button |
There was a problem hiding this comment.
it's a bit of a small nicety, but i like putting an autoFocus on the close button when there is one. webaim suggests this as a requirement for a11y, though that can get a little iffy when you're somewhere between a dialog and an "expanded view". my rule of thumb has generally been "if there's an X, just focus it". would be great to do the inverse too (refocus the source row on close)
fakemonster
left a comment
fakemonster
left a comment
There was a problem hiding this comment.
a few more observations. just like in social media, infinite scroll leads to pain!
| maxValue: now(getLocalTimeZone()), | ||
| }) | ||
|
|
||
| const { intervalPicker } = useIntervalPicker({ |
There was a problem hiding this comment.
having returned to this view... do we actually want an interval picker? right now if i load more pages and then the interval fires, pages 2 and on are just dropped, which seems all too surprising. we could fix that in one of two ways:
- when a refresh fires, only prepend those new incoming logs to the existing set. this would involve searching for the intersection with the existing list, dropping the redundant ones, and correcting the selected index if there is one. kind of fragile. plus, if the new page doesn't intersect with the logs we already have, we'll need to automatically make further requests to fill the gap
- let the refresh wipe state, but be "however many pages you had rendered before". that's a little odd cause some logs are still potentially disappearing. and since we're giving an infinite query here, we're effectively saying "go ahead and load a few dozen pages, and we'll refresh all of them on an interval". certainly there's a limit somewhere?
if we had only only explicit reload, it would make dropping pages 2+ much more expected, imo. or we could just let good old full-page refreshing/time period changes be the refresh pattern here
The previous attempt to read an error message would never succeed, and since errors are generally happening at the bottom of the page, I moved the error message down there. If we get rid of interval refreshing, _all_ errors will be down there!
4 participants
Copied from #2849, which I accidentally merged and couldn't reopen even after I fixed
main.Still very rough, but has been a helpful exercise in working through some of the design due to the sheer amount of information and differing layout from other pages.
!https://github.com/user-attachments/assets/84cbb9f6-d22b-4e64-9b1d-e39d74d22902
Stubbing out based on oxidecomputer/omicron#7339.
Uses Tanstack Virtual. On testing with > 500 lines without virtualisation it starts to get a bit chunky especially if you're interacting with the page (e.g. opening the row).
Hoping that silo name and actor display name can be plumbed through so those are hard-coded for now.
Still needs: