Skip to content
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

fix: Enhancement of Visual Json Schema Editor #1065

Open
wants to merge 35 commits into
base: master
Choose a base branch
from

Conversation

Gmin2
Copy link

@Gmin2 Gmin2 commented Apr 5, 2024

Description

  • [✅ ] Added Delete Property Function
  • [✅ ] Changed Type of Property Function
  • [✅] Toggle Required/NotRequired Function
  • [ ✅] UI
  • [✅ ] Added Property function, but only for cases which have a path containing array

    Related issue(s)
    Resolves Enhance the Visual JSON Schema Editor  #1023

Copy link

changeset-bot bot commented Apr 5, 2024

⚠️ No Changeset found

Latest commit: 7b4fb9d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

netlify bot commented Apr 5, 2024

Deploy Preview for modest-rosalind-098b67 ready!

Name Link
🔨 Latest commit 7b4fb9d
🔍 Latest deploy log https://app.netlify.com/sites/modest-rosalind-098b67/deploys/6658a4656503190008bcac4a
😎 Deploy Preview https://deploy-preview-1065--modest-rosalind-098b67.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Apr 5, 2024

Deploy Preview for asyncapi-studio-design-system ready!

Name Link
🔨 Latest commit 7b4fb9d
🔍 Latest deploy log https://app.netlify.com/sites/asyncapi-studio-design-system/deploys/6658a465964d9d00084167cd
😎 Deploy Preview https://deploy-preview-1065--asyncapi-studio-design-system.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

Copy link

netlify bot commented Apr 5, 2024

Deploy Preview for studio-next ready!

Name Link
🔨 Latest commit 7b4fb9d
🔍 Latest deploy log https://app.netlify.com/sites/studio-next/deploys/6658a465d763680008b7697b
😎 Deploy Preview https://deploy-preview-1065--studio-next.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Gmin2 Gmin2 marked this pull request as draft April 5, 2024 19:29
@Gmin2 Gmin2 changed the title Test editor fix: Enhancement of Visual Json Schema Editor Apr 5, 2024
@Gmin2 Gmin2 marked this pull request as ready for review April 12, 2024 21:34
@Gmin2
Copy link
Author

Gmin2 commented Apr 12, 2024

Hey @princerajpoot20 it is done and ready for review

cc @Amzani

Copy link
Member

@princerajpoot20 princerajpoot20 left a comment

Choose a reason for hiding this comment

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

Good work @utnim2 .

Have you tested this with all the sample JSON schemas?

  • There is no option to add property to the array<object> type.
  • In the UI for array properties, we should not have Array Items as text.
  • If we are changing the type of an existing property, when I change it to type array of string, it directly assigns the same value. For example: "type": "array<string>".

Instead, it should be like

"type": "array",
  "items": {
    "type": "number"
  }

Could you make the code editor text be beautify after performing any operation

… removed "Array Items", fixed changing of type for array types
@Gmin2
Copy link
Author

Gmin2 commented Apr 15, 2024

Done all the reviewed changes @princerajpoot20

Could you make the code editor text be beautify after performing any operation

could you explain this more what exactly do you want, ig i have matched the design provided

@princerajpoot20
Copy link
Member

Screenshot 2024-04-16 at 7 10 40 PM Screenshot 2024-04-16 at 7 10 50 PM

@utnim2 After modification, the JSON schema is not in a readable form.

  • In the schema shown in Figma, the friends array can have two data types: integer or null. We should make it accordingly.
  • In the above case, if we consider creating this JSON schema using a visual editor, we currently cannot make the friends array support more than one value. To address this, we could add a multiselect checklist or something similar in the add property field, allowing us to select two data types when adding a property.
  • Some UI changes are still needed, such as the colors for arrays and objects being different in array<object>, and the colors, font size and font style are not exactly matching those in Figma.
Screenshot 2024-04-16 at 7 23 27 PM

After having array , in next line we again mention object.

Screenshot 2024-04-16 at 7 24 09 PM

This text is there if root is of type array

Screenshot 2024-04-16 at 7 24 22 PM

This text can be removed

@Gmin2
Copy link
Author

Gmin2 commented May 22, 2024

@Gmin2 deploy is failing. Could you check that?

fixed 👍

KhudaDad414
KhudaDad414 previously approved these changes May 24, 2024
Copy link
Member

@KhudaDad414 KhudaDad414 left a comment

Choose a reason for hiding this comment

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

LGTM

@princerajpoot20
Copy link
Member

princerajpoot20 commented May 25, 2024

@Gmin2, Good work! Just a few changes:

  1. We are not able to build the JSON schema from scratch.
    • In the case of an empty JSON schema {}, we should only have the option to define the root type. We should not have the add property option there. It should be there only for the type defined as object or array-object.

Like for example:

{
      "type": "string"
}

In this JSON schema, we will not have an add property option, as we cannot add a property anywhere in this JSON schema since the root is of type string.

  1. The add property option is not working correctly in cases where the root is of type array-object. If we try to add a property to that schema, it changes the root type from array-object to object.

  2. The UI doesn't support selecting multiple data types while adding a new property.

  3. After modification from the visual editor, the code editor (JSON schema) is not in a readable form.

You can add these JSON schemas to the stories as well.⬇︎
  1. Blank Json Schema
{
      
}
  1. Root type string
{
      "type": "string"
}
  1. Property having multiple data types
{
      "type": "object",
      "properties": {
        "mixedTypeProperty": {
          "type": ["boolean", "integer"]
        }
      },
      "required": ["mixedTypeProperty"]
}

@Gmin2
Copy link
Author

Gmin2 commented May 27, 2024

The UI doesn't support selecting multiple data types while adding a new property.

as we are using radix Dropdown component (can you check this), not sure if we can implement it, we might have a workaround but i am constantly trying with introducing state but multi-select is not working.

After modification from the visual editor, the code editor (JSON schema) is not in a readable form.

image
it is same as was before the changes that i have made

cc @princerajpoot20

@princerajpoot20
Copy link
Member

princerajpoot20 commented May 27, 2024

it is same as was before the changes that i have made

I have tested this through the Netlify deploy link in the PR, there it is not rendering in a readable form.

Screenshot 2024-05-27 at 11 17 24 PM

@Gmin2
Copy link
Author

Gmin2 commented May 27, 2024

I have tested this through the Netlify deploy link in the PR, there it is not rendering in a readable form.

netlify is not rendering the latest commits, you can use github cli and then see the changes

@princerajpoot20
Copy link
Member

princerajpoot20 commented May 27, 2024

It is not that Netlify has not rendered your latest commit. It automatically takes your latest commit. Their problem is something else.

Screenshot 2024-05-28 at 1 56 06 AM

If the changes are not visible in the PR preview, then they will not be visible in deployment (after merge) either.

@Gmin2
Copy link
Author

Gmin2 commented May 28, 2024

If the changes are not visible in the PR preview, then they will not be visible in deployment (after merge) either.

it is working fine in my machine, idk why netlify is parsing it as a string rather then schemaObject

@Gmin2
Copy link
Author

Gmin2 commented May 29, 2024

Hey @princerajpoot20 in the deploy preview also it is showing right

image

@princerajpoot20
Copy link
Member

@Gmin2 I'm talking about editor after making changes through visual editor.
Ref::

screen.recording.mov

@KhudaDad414
Copy link
Member

@princerajpoot20 is that necessary? we will have something nicer like a Codemirror editor and an easy-to- edit format like YAML over there anyway.

Copy link
Member

@KhudaDad414 KhudaDad414 left a comment

Choose a reason for hiding this comment

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

LGTM!

packages/ui/components/DropdownMenu.tsx Show resolved Hide resolved
packages/ui/components/DropdownMenu.tsx Show resolved Hide resolved
@princerajpoot20
Copy link
Member

@princerajpoot20 is that necessary? we will have something nicer like a Codemirror editor and an easy-to- edit format like YAML over there anyway.

@KhudaDad414 Sure, this sounds good.

KhudaDad414
KhudaDad414 previously approved these changes May 29, 2024
Copy link

sonarcloud bot commented May 30, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.1% Duplication on New Code

See analysis details on SonarCloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty AsyncAPI Bounty
Projects
Status: In Progress
Status: In progress
Development

Successfully merging this pull request may close these issues.

Enhance the Visual JSON Schema Editor
6 participants