-
Notifications
You must be signed in to change notification settings - Fork 2
properly render empty objects #18
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
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.
Hello @MichaelDoyle, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
As gemini-code-assist, I've reviewed this pull request by MichaelDoyle. The primary goal of this PR, as indicated by the title and description (Fixes #13), is to correctly handle and render empty objects ({}) within the ngx-json-treeview component. It appears that previously, empty objects might not have been displayed as intended when treated as simple, non-expandable values. This change modifies the component's rendering logic for such values to use JSON.stringify for a more robust string representation and includes an update to the demo application to showcase the rendering of an empty object.
Highlights
- Fix Empty Object Rendering: The core issue of incorrectly rendering empty objects is addressed by changing the display logic for simple JSON values (primitives, null, empty objects/arrays). These values are now converted to a formatted string using
JSON.stringifybefore being displayed. - Update Component Template: The
ngx-json-treeview.component.htmltemplate is updated in the section handling non-segmented values (lines 2-8) to call a newasString()method instead of directly interpolating thejson()signal with a nullish coalescing operator. - Introduce
asStringComputed Property: A newcomputedproperty namedasStringis added to theNgxJsonTreeviewComponentclass (line 145). This property usesJSON.stringify(this.json(), null, 2)to generate a formatted string representation of the input JSON data. - Add Demo Case for Empty Object: The demo application's
app.component.htmlis updated (line 43) to include a new instance of thengx-json-treeviewcomponent specifically rendering an empty object ({}), providing a visual test case for the fix.
Changelog
- projects/demo/src/app/app.component.html
- Added a new
ngx-json-treeviewinstance to display an empty object{}under the 'Primatives' section (line 43).
- Added a new
- projects/ngx-json-treeview/src/lib/ngx-json-treeview.component.html
- Modified the template to use the
asString()method for rendering simple JSON values (line 5).
- Modified the template to use the
- projects/ngx-json-treeview/src/lib/ngx-json-treeview.component.ts
- Added a
computedpropertyasStringthat returns theJSON.stringifyrepresentation of the input JSON (line 145).
- Added a
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
JSON tree view,
Empty object now shows,
Stringify helps.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request effectively addresses the issue of improperly rendered empty objects by introducing a new asString computed signal that uses JSON.stringify. The changes are clear, well-targeted, and improve the display of primitive-like root JSON values, including empty objects and arrays. The addition of a test case in the demo application is also a good practice.
Overall, the code quality is good, and the use of Angular signals is appropriate. I have one suggestion regarding the asString implementation for potential future robustness, but the current solution successfully fixes the main problem.
Summary of Findings
- Rendering of Empty Objects/Arrays: The PR successfully fixes the rendering of empty objects and arrays when they are the root JSON input, changing from potentially
[object Object]or""to{ }and[ ]respectively. - JSON Stringification for Primitives: The
asStringcomputed property now usesJSON.stringify(this.json(), null, 2). A suggestion was made to considerJSON.stringify(this.json())for more robust single-line rendering of empty objects/arrays in the primitive display context, though the current approach works well with default HTML whitespace handling.
Merge Readiness
The pull request is in good shape and effectively resolves the stated issue. The code changes are clear and improve the component's rendering. There is one medium-severity suggestion for consideration regarding the stringification method, but it's not a blocker for merging. I am unable to approve pull requests, but based on this review, the changes look good to go after considering the feedback. Please ensure other necessary reviews and approvals are obtained.
| return ']'; | ||
| } else return '}'; | ||
| }); | ||
| asString = computed<string>(() => JSON.stringify(this.json(), null, 2)); |
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.
The new asString computed property correctly uses JSON.stringify to provide a string representation for root values when no segments are generated (e.g., for primitives, empty objects, or empty arrays). This fixes the issue where empty objects might have rendered as [object Object].
The use of JSON.stringify(this.json(), null, 2) introduces newlines and indentation (e.g., {\n \n} for an empty object). Currently, HTML's default whitespace collapsing behavior renders this as { }, which is a good outcome.
However, for improved robustness against potential future CSS changes (e.g., if .segment-primative or its container ever gets white-space: pre or white-space: pre-wrap), this could lead to multi-line displays for these empty structures, which might not be ideal for this compact "primitive" view.
Have you considered using JSON.stringify(this.json()) (without the null, 2 arguments) for this specific asString computed property? This would produce compact strings like "{}" and "[]" directly, ensuring a single-line representation irrespective of white-space styling, while still correctly rendering other primitives like strings, numbers, and booleans in their JSON string form.
| asString = computed<string>(() => JSON.stringify(this.json(), null, 2)); | |
| asString = computed<string>(() => JSON.stringify(this.json())); |
Fixes #13