Skip to content

Comments

feat(ui): allow JSONB insert and adding the collapsable JSON#937

Merged
daryllimyt merged 17 commits intoTracecatHQ:mainfrom
Jarro01X:fix/json-table-insert-and-view
Mar 25, 2025
Merged

feat(ui): allow JSONB insert and adding the collapsable JSON#937
daryllimyt merged 17 commits intoTracecatHQ:mainfrom
Jarro01X:fix/json-table-insert-and-view

Conversation

@Jarro01X
Copy link
Contributor

@Jarro01X Jarro01X commented Mar 11, 2025

Checklist

  • Read CONTRIBUTING.md.
  • PR title is short and non-generic (see previously merged PRs for examples).
  • PR only implements a single feature or fixes a single bug.
  • Tests passing (uv run pytest tests)?
  • Lint / pre-commits passing (pre-commit run --all-files)?

Description

Adding a fix that makes it possible to add JSONB to tables, and the ability to collapse or retract items longer then 15 lines

Related Issues

Fixes #931

Screenshots

See comments below

Steps to QA

  1. Go to tables
  2. Create a table with JSONB column
  3. Add a valid JSONB
  4. Make sure it is longer then 15 lines in case you want to test collapse and retractability

Copy link
Contributor

@daryllimyt daryllimyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comments suggesting we reuse the json viewer we've been using all this while. Would be good to have your thoughts here too @topher-lo

@topher-lo
Copy link
Contributor

Agreed on the JSON viewer. Also we should consider only showing prettified JSON if expanded. Otherwise the tables will get too with <5-10 rows and large nested jsons (which is expected for LLM memory and security related logs)

@Jarro01X
Copy link
Contributor Author

Jarro01X commented Mar 13, 2025

oh sorry, I hadn't noticed you guys were using that. Let me go ahead and make the adjustments

@topher-lo topher-lo added the enhancement New feature or request label Mar 13, 2025
@topher-lo topher-lo changed the title fix(ui): allow JSONB insert and adding the collapsable JSON feat(ui): allow JSONB insert and adding the collapsable JSON Mar 13, 2025
@topher-lo topher-lo added the fix Bug fix label Mar 13, 2025
@Jarro01X Jarro01X requested a review from daryllimyt March 13, 2025 19:22
@Jarro01X
Copy link
Contributor Author

Jarro01X commented Mar 13, 2025

@topher-lo done, the JSON gets prettified very nicely if you choose the Nested option. This is probably some of the best UI/UX I've ever seen for viewing JSONs, this is great! Much better now honestly

Screenshot_107

@Jarro01X
Copy link
Contributor Author

Alright there we go, everything is running well on my end. The branch hadn't been updated so running the new/modified tests that are in the CI/CD pipeline wasn't working

@topher-lo
Copy link
Contributor

Looking good. Just waiting on @daryllimyt for final review

Copy link
Contributor

@daryllimyt daryllimyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a couple of small adjustments

import { TableViewAction } from "@/components/tables/table-view-action"
import { TableViewColumnMenu } from "@/components/tables/table-view-column-menu"
import { JsonViewWithControls } from "../workbench/events/events-selected-action"
import { TooltipProvider } from "@radix-ui/react-tooltip"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer shadcn tooltip over the raw radix one

Suggested change
import { TooltipProvider } from "@radix-ui/react-tooltip"
import { TooltipProvider } from "@/components/ui/tooltip"

import { DataTable } from "@/components/data-table"
import { TableViewAction } from "@/components/tables/table-view-action"
import { TableViewColumnMenu } from "@/components/tables/table-view-column-menu"
import { JsonViewWithControls } from "../workbench/events/events-selected-action"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we:

  1. Move JsonViewWithControls into a separate file under /components/json-viewer.tsx
  2. Import it in thie file and in /workbench/events/events-selected-action using its absolute path prefixed with @ i.e. @/components/json-viewer.tsx

@daryllimyt daryllimyt self-requested a review March 24, 2025 13:45
Copy link
Contributor

@daryllimyt daryllimyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚢 ! Just some linter errors and it's good to go. Great work @Jarro01X !

@daryllimyt daryllimyt merged commit d02c46f into TracecatHQ:main Mar 25, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request fix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot add JSONB value to lookup table via UI

3 participants