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

🐛 Bug: Unintentional behaviour of the code snippet section #417

Open
AgniveshChaubey opened this issue Mar 3, 2024 · 10 comments
Open
Assignees
Labels
🐛 Bug Indicates that the issue is a bug or defect. Status: On Hold Similar to blocked, but is assigned to someone.

Comments

@AgniveshChaubey
Copy link

AgniveshChaubey commented Mar 3, 2024

Schema code snippet

When copying the schema using the copy to clipboard button, the title of that section in the top right corner changes to data (from schema) for a moment. I'm not sure if it is intentional, but it should not change I guess.

image

After copying:

image


Instance code snippet

When copying the instance using the copy to clipboard button, the bottom row showing the validation result disappears for a moment, which also should not happen.
image

After copying:
image

Expected Behavior

  1. schema should not change to data when copying the schema snippet.
  2. The bottom row should not disappear when copying the Instance snippet.
@AgniveshChaubey AgniveshChaubey added the 🐛 Bug Indicates that the issue is a bug or defect. label Mar 3, 2024
@github-actions github-actions bot added the Status: Triage This is the initial status for an issue that requires triage. label Mar 3, 2024
@DhairyaMajmudar
Copy link
Member

If approved , I would like to take this up.

@benjagm benjagm added Status: Available No one has claimed responsibility for resolving this issue. and removed Status: Triage This is the initial status for an issue that requires triage. labels Mar 3, 2024
@benjagm
Copy link
Collaborator

benjagm commented Mar 3, 2024

Thanks Agnivesh!! @DhairyaMajmudar please go ahead.

@benjagm benjagm added Status: In Progress This issue is being worked on, and has someone assigned. and removed Status: Available No one has claimed responsibility for resolving this issue. labels Mar 3, 2024
@DhairyaMajmudar
Copy link
Member

@benjagm @AgniveshChaubey I think the schema changing to data is an intentional behavior since while analysing the code file I found that both of them are wrapped in a ternary conditional block depending on variable isJsonSchema

Pls. give your views on this.

@benjagm
Copy link
Collaborator

benjagm commented Mar 4, 2024

I think the label should stay consistent after copying data.

@AgniveshChaubey
Copy link
Author

AgniveshChaubey commented Mar 4, 2024

I think the schema changing to data is an intentional behavior since while analysing the code file I found that both of them are wrapped in a ternary conditional block depending on variable isJsonSchema

I haven't gone through the codebase of the website, but when quickly looking at the file you mentioned, it seems that this ternary operator is to check whether the snippet is a JSON Schema or a JSON data

          <div className='flex flex-row items-center text-white h-6 font-sans bg-white/20 text-xs px-3 rounded-bl-lg font-semibold'>
            {isJsonSchema ? (
              <>
                <img src='/logo-white.svg' className='h-4 mr-1.5' /> schema
              </>
            ) : (
              <>data</>
            )}
          </div>

Edit:
And when it is JSON Schema render

              <>
                <img src='/logo-white.svg' className='h-4 mr-1.5' /> schema
              </>

and when it is JSON data, render

<>data</>

@benjagm
Copy link
Collaborator

benjagm commented Mar 11, 2024

Dhairya are you working on this?

@DhairyaMajmudar
Copy link
Member

Yes I am trying to find its solution its taking time because its very anomalous , sometime the bug appears and sometimes not.

@DarhkVoyd
Copy link
Member

DarhkVoyd commented Mar 18, 2024

@DhairyaMajmudar I am not sure but I believe it has something to do with how we are implementing the copy to clipboard component. Currently, instead of being a separate component with its own state, we are setting the copied state on the entire JsonEditor component due to which after each copy the Json Editor is caused to re-render again and again multiple times simply due to a copy event. Since, its somewhat heavy component with calculations, react would schedule updates and they would be async and in no particular order in some cases the state change would re-render but the data might not be ready and therefore it breaks to dynamically assert if the data is schema in some renders and valid or invalid in others, causing these errors.

We should probably refactor the component to move the state from the root JsonEditor component even if that doesn't solve the issue for a better state management and efficient component.

@benjagm
Copy link
Collaborator

benjagm commented Mar 18, 2024

Thanks everyone! Great inputs here. I think the path forward is creating a new issue to review and refine the current code editor strategy. To do it more consistently.

@benjagm benjagm added Status: On Hold Similar to blocked, but is assigned to someone. and removed Status: In Progress This issue is being worked on, and has someone assigned. labels Mar 18, 2024
@benjagm
Copy link
Collaborator

benjagm commented Mar 18, 2024

I am putting this on hold in favour of #560

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Indicates that the issue is a bug or defect. Status: On Hold Similar to blocked, but is assigned to someone.
Projects
Status: On Hold
Development

No branches or pull requests

4 participants