feat(logs): drawer with details #2155
Conversation
|
Next PRs:
|
| res.status(400).send({ | ||
| error: { code: 'invalid_body', errors: zodErrorToHTTP(val.error) } | ||
| }); | ||
| return; |
There was a problem hiding this comment.
those 2 checks come back regularly. I imagine we could abstract them
There was a problem hiding this comment.
I think using your wrapper might be the best solution because I could not come up with an abstraction that save more than 1 line (you still need to return something so you still need a if)
but if you have something in the meantime, happy to change
There was a problem hiding this comment.
I don't have a concrete solution right now. my comment was more to keep it in the back on our mind so at some point we come up with something
| const body: SearchMessages['Body'] = val.data; | ||
| const rawOps = await model.listMessages({ | ||
| parentId: body.operationId, | ||
| limit: body.limit!, |
There was a problem hiding this comment.
I wonder if there is a way make the type reflect the fact that after validation, since limit has a default value it cannot be undefined anymore
There was a problem hiding this comment.
yes it's a bit annoying, with types you often need to produce multiple variants: before/after validation, before/after creation, etc.
I'm not sure if there is a good answer expect maybe having a Body variant in the Endpoint types that you can alter to reflect post-validation type.
Yes sorry, I added the UI componennt and realized it was more complicated than anticipated |
| <Route path="/:env/environment-settings" element={<EnvironmentSettings />} /> | ||
| <Route path="/:env/project-settings" element={<Navigate to="/environment-settings" />} /> | ||
| }} | ||
| > |
There was a problem hiding this comment.
hard to tell what this is changing....what is the adjustment here?
There was a problem hiding this comment.
I added a wrapper for the tooltip provider. You can see those diff better with "whitespace" off
| return ( | ||
| <div className="py-6 px-6 flex flex-col gap-9"> | ||
| <h3 className="text-xl font-semibold text-white flex gap-4 items-center">Operation Details</h3> | ||
| <Skeleton className="h-4 w-[250px]" /> |
There was a problem hiding this comment.
If the Skeleton spacing needs to change seems it'll be annoying to change it in all these different places
There was a problem hiding this comment.
by spacing you mean height?
There was a problem hiding this comment.
yeah, height and weight. My comment should have said:
If the Skeleton element needs to change
There was a problem hiding this comment.
Skeleton is made to be flexible to reflect the final UI so it's bound to be modified where it needs to. I'll set the default height which is more common but the width is context specific.
For the ref, this is that: https://ui.shadcn.com/docs/components/skeleton
| <div className="font-semibold text-sm">Connection</div> | ||
| <div className="text-gray-400 text-s font-code truncate"> | ||
| {operation.connectionName ? ( | ||
| <Link to={`/connections/${operation.connectionName}`} target="_blank" className="flex gap-1 items-center hover:text-white"> |
There was a problem hiding this comment.
Should it be a new link? I'm not sure, asking from a product perspective though
There was a problem hiding this comment.
new link === opens in a new tab
There was a problem hiding this comment.
ah yes sorry I see. The main inspiration is datadog drawer so for now it's the intended behavior to not loose the context. There are plenty of stuff in the UX that will evolve with the first feedback
| }, | ||
| width: { | ||
| largebox: '1200px', | ||
| largecell: '480px' | ||
| }, | ||
| fontSize: { | ||
| s: '13px', |
There was a problem hiding this comment.
14px is also used a lot, seems a bit strange for things to go to 13px. Something to confirm with the design I suppose
There was a problem hiding this comment.
Yes, we increased the font-size but I think 14 was too big. You can find the figma in Product > Logs V2
Describe your changes
Fixes NAN-847
Contributes to NAN-908
Rename
POST /logs/searchtoPOST /logs/operationsIt was too different after all for a unified endpoint
Create
POST /logs/messagesto search in messages tied to an operationImplement Operation's Drawer
Implement Messages's Drawer
Full text search on logs
http://localhost:3000/dev/logs
Screen.Recording.2024-05-21.at.11.13.23.mp4