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

Merged
merged 39 commits into from
Jun 6, 2024

Conversation

Gmin2
Copy link
Contributor

@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: 7707e06

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 7707e06
🔍 Latest deploy log https://app.netlify.com/sites/modest-rosalind-098b67/deploys/66603d5598d3bd0008b8c8e0
😎 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 7707e06
🔍 Latest deploy log https://app.netlify.com/sites/asyncapi-studio-design-system/deploys/66603d55f26f9a0009f1919a
😎 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 7707e06
🔍 Latest deploy log https://app.netlify.com/sites/studio-next/deploys/66603d554ee7720008445c92
😎 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
Contributor 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
Contributor 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
Contributor 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
Contributor 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
Contributor 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
@KhudaDad414 KhudaDad414 mentioned this pull request May 29, 2024
6 tasks
@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
KhudaDad414
KhudaDad414 previously approved these changes Jun 3, 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

@KhudaDad414
Copy link
Member

@princerajpoot20 pingy pongy.

princerajpoot20
princerajpoot20 previously approved these changes Jun 4, 2024
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.

LGTM

@Gmin2 Gmin2 dismissed stale reviews from princerajpoot20 and KhudaDad414 via 3648dc0 June 4, 2024 21:22
Copy link

sonarcloud bot commented Jun 5, 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

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.

LGTM

@Gmin2 Gmin2 requested a review from KhudaDad414 June 6, 2024 10:02
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

@Gmin2
Copy link
Contributor Author

Gmin2 commented Jun 6, 2024

/rtm

@asyncapi-bot asyncapi-bot merged commit 2a3b710 into asyncapi:master Jun 6, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Completed
Status: Done
Development

Successfully merging this pull request may close these issues.

Enhance the Visual JSON Schema Editor
6 participants