Skip to content

feature/json viewer - #174

Open
Dustin Pham (DustinPham) wants to merge 9 commits into
mainfrom
feature/json-viewer
Open

feature/json viewer#174
Dustin Pham (DustinPham) wants to merge 9 commits into
mainfrom
feature/json-viewer

Conversation

@DustinPham

@DustinPham Dustin Pham (DustinPham) commented Feb 19, 2026

Copy link
Copy Markdown
Contributor

implementation of json-viewer in angular and react

TODO:

  • Add figma code connect when component is made

Signed-off-by: Dustin Pham <dustinpham95@gmail.com>
Signed-off-by: Dustin Pham <dustinpham95@gmail.com>
Signed-off-by: Dustin Pham <dustinpham95@gmail.com>
Signed-off-by: Dustin Pham <dustinpham95@gmail.com>
Signed-off-by: Dustin Pham <dustinpham95@gmail.com>

@R-Bower Ryan Bower (R-Bower) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At a first glance, this looks promising. I'll do a more detailed review this week.

@R-Bower Ryan Bower (R-Bower) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A few small requests, nothing major. Overall, really good.

I'm going to hold off on merging this until we've aligned with the QDS team on a syntax highlighting theme. Details here.

Comment thread packages/frameworks/react/src/json-viewer/json-viewer.tsx Outdated
Comment thread packages/frameworks/react/src/json-viewer/json-viewer-node.tsx
Comment thread packages/common/qds-core/src/json-viewer/qds-json-viewer.css Outdated

@R-Bower Ryan Bower (R-Bower) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Image

ah yes, fonUpdate()

We should probably distinguish the function indicator from the function name. Probably use a different color

@R-Bower

Copy link
Copy Markdown
Contributor

slight shift on the expand/collapse demo, easy fix
2026-03-03 00 42 12

Comment thread packages/frameworks/react/src/json-viewer/json-viewer-root.tsx Outdated
Comment thread packages/frameworks/react/src/json-viewer/json-viewer-key-node.tsx Outdated
Comment thread packages/frameworks/react/src/json-viewer/json-viewer-value-node.tsx Outdated
callback: function onUpdate() {},
errors: [
new Error("Connection timeout"),
new TypeError("Expected string, received number"),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

interesting... this is causing a hydration issue.

Image
Uncaught Error: Hydration failed because the server rendered text didn't match the client. As a result this tree will be regenerated on the client. This can happen if a SSR-ed Client Component used:

- A server/client branch `if (typeof window !== 'undefined')`.
- Variable input such as `Date.now()` or `Math.random()` which changes each time it's called.
- Date formatting in a user's locale which doesn't match the server.
- External changing data without sending a snapshot of it along with the HTML.
- Invalid HTML tag nesting.

This is probably because the TypeError object is created on both the client and server, and it's a separate object on each. I wonder if we need to account for this, re: SSR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed .stack from the errors seemed to fix this, just a demo workaround though

- add qds api for data bindings
- use new qds api in angular and react key/value nodes
- use correct font
- fix hydration issue with SSR by removing stack from errors created for demo
- change JsonViewerSimpleProps to JsonViewerProps for react implementation
- Make f color for functions neutral grey

Signed-off-by: Dustin Pham <dustinpham95@gmail.com>
@R-Bower Ryan Bower (R-Bower) added the Waiting on Design Indicates that the task is dependent on in-progress work from the design team label Mar 30, 2026
@R-Bower

Copy link
Copy Markdown
Contributor
image

If possible I'd like to remove this part when the element is expanded

return <>{renderValue?.(node) ?? node.value}</>
}

const Element = node.tagName

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I checked the utils folder and node.tagName is only ever span. We can probably remove this property altogether. I don't ever see it being anything else.

provideQdsTreeContext(),
provideJsonViewerOptionsContext(),
],
selector: "q-json-viewer-root",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be a directive. We can use the trackBindings approach like we do in other components.

@github-actions

Copy link
Copy Markdown

This pull request has been marked as stale due to 60 days of inactivity. To prevent automatic closure in 10 days, remove the stale label or add a comment. You can reopen a closed pull request at any time.

@github-actions

Copy link
Copy Markdown

This pull request has been marked as stale due to 60 days of inactivity. To prevent automatic closure in 10 days, remove the stale label or add a comment. You can reopen a closed pull request at any time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Waiting on Design Indicates that the task is dependent on in-progress work from the design team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants