Skip to content

Conversation

@severo
Copy link
Contributor

@severo severo commented Apr 23, 2025

fixes #220

  • missing: fix existing tests and add new ones
Screencast.From.2025-04-23.23-33-33.mp4

@severo severo changed the title 220 use less space for array of numbers in json view Collapse arrays and objects in json view Apr 23, 2025
@severo
Copy link
Contributor Author

severo commented Apr 23, 2025

feel free to try it and give your comments about it, before I finish the tests @platypii

@platypii
Copy link
Contributor

slick! I like it

@severo severo force-pushed the 220-use-less-space-for-array-of-numbers-in-json-view branch from 1c3746b to fc97856 Compare April 24, 2025 09:29
@severo severo force-pushed the 220-use-less-space-for-array-of-numbers-in-json-view branch from c085223 to 3e0e475 Compare April 24, 2025 09:59
@severo severo marked this pull request as ready for review April 24, 2025 10:19
@severo severo requested review from bleakley, Copilot and platypii April 24, 2025 10:19
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds functionality to collapse arrays and objects in the JSON viewer while updating tests and stories to reflect the new behavior.

  • Refactored JSON components to introduce collapsible views for arrays and objects.
  • Added helper functions (isPrimitive and shouldObjectCollapse) to control collapse behavior.
  • Updated test cases and Storybook stories to verify the new collapsing logic.

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/components/JsonView/JsonView.test.tsx Adjusted tests to validate rendering of nested lists with collapsible behavior.
src/components/Json/helpers.ts Introduced helper functions to determine when to collapse objects.
src/components/Json/Json.tsx Refactored components to use new collapsing logic for arrays and objects.
src/components/Json/Json.test.tsx Expanded tests to cover various collapse/expand scenarios for arrays and objects.
src/components/Json/Json.stories.tsx Added Storybook stories to demonstrate the updated JSON view with collapsible elements.
Files not reviewed (1)
  • src/components/Json/Json.module.css: Language not supported
Comments suppressed due to low confidence (3)

src/components/Json/Json.tsx:3

  • Ensure that the import path './helpers.js' aligns with your TypeScript module resolution configuration. Consider removing the file extension if your project is set up to resolve TypeScript files automatically.
import { isPrimitive, shouldObjectCollapse } from './helpers.js'

src/components/Json/Json.tsx:59

  • Review the inequality condition here; if the intention is to include cases where characterCount equals maxCharacterCount, consider changing '<' to '<='.
if (characterCount < maxCharacterCount) {

src/components/Json/Json.tsx:123

  • Similarly for CollapsedObject, consider including the boundary case by using '<=' instead of '<' to handle exact character count matches.
if (characterCount < maxCharacterCount) {

Copy link
Contributor

@bleakley bleakley left a comment

Choose a reason for hiding this comment

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

This is very nice, one minor nit is that even though null is an object type I think it could make sense to treat them as primitives in this case

@severo
Copy link
Contributor Author

severo commented Apr 24, 2025

good idea, I'll consider null as a primitive (it IS a primitive, by the way: https://developer.mozilla.org/en-US/docs/Glossary/Primitive, as undefined and ... symbol)

@severo
Copy link
Contributor Author

severo commented Apr 24, 2025

I added undefined and null as primitives + fixed some issues with style (color in arrays for example)

Screenshot From 2025-04-24 22-52-58
Screenshot From 2025-04-24 22-52-50

Copy link
Contributor

@bleakley bleakley left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

@severo severo merged commit 0d67c6e into master Apr 24, 2025
4 checks passed
@severo severo deleted the 220-use-less-space-for-array-of-numbers-in-json-view branch April 24, 2025 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use less space for arrays of numbers in JSON view

4 participants