fix: minor bugs in right pane

[harish-sethuraman]

Prerequisites checklist

• I have read the contributing guidelines.

What is the purpose of this pull request?

fixes #37

What changes did you make? (Give an overview)

• added an isArray check to its parent elements

Related Issues

#37

Is there anything you'd like reviewers to focus on?

• Many of them have "Unknown" in the heading - we assume the value to be one of three - Scope, Reference, Variable if its apart from these we show unknown. Do we see any other values apart from these.
• Some of them are rendered as JSON - These are hardwired to code logic. If the values are array of values rather than array of objects its renderered, if the value is object and the values of the objects aren't nestable then we render them as code. If the values of array or object arent nestable to next level then we show them as JSON directly
• Many have "0." in front of the type in the heading even when not in an array ✅
• Array items all show "0" in front of the heading type ✅

[eslint-github-bot]

Hi @harish-sethuraman!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

• The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
• There should be a space following the initial tag and colon, for example 'feat: Message'.
• The first letter of the tag should be in lowercase

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

[netlify]

✅ Deploy Preview for eslint-code-explorer ready!

Name	Link
🔨 Latest commit	99c44d9
🔍 Latest deploy log	https://app.netlify.com/sites/eslint-code-explorer/deploys/66ee498f91855b000893ad18
😎 Deploy Preview	https://deploy-preview-43--eslint-code-explorer.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

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

[nzakas]

It would be helpful if you could format the todo list as a checklist so it's easier to tell what's completed.

[harish-sethuraman]

Updated

[nzakas]

Just a heads up that there's a conflict now.

[nzakas]

@harish-sethuraman these fixes are the last thing preventing us from launching, so can you let us know what your timetable is for working on this? I'm happy to jump in and take a look, too.

[harish-sethuraman]

Yes I might need help in deciding the first two points.

1. What do we want to show there 🤔 feel free to check once at the implementation there.
2. I don't think we might want to change this logic it is good as it is? Or are you expecting code mirror instances for these values as well? instead of rendering it inside pre?

[nzakas]

1. The heading should be the value of the type property of an AST node. Otherwise just Object.
2. We don't want to show JSON in the tree view. Every object should be represented the same way.

[harish-sethuraman]

1. have fixed this
2. is this UI fine for empty values?

[harish-sethuraman]

Also I was able to see that some accordions would trigger other ones as the key were same. Now I have generated unique paths to these accordions.

[nzakas]

This is looking better. Some points:

1. The value in the accordion heading should be the value of node.type is present. So "0. Object" should be "0.Member" in the following screenshot.
2. Similarly, when collapsed, we should use node.type when present. So "name Object" should be "name String".
   

[harish-sethuraman]

will take some time on this. I might have to dig a bit for these two.

[nzakas]

If you want, we can merge this and then I can help look into it.

[harish-sethuraman]

Yep sure. I can look into this in a separate issue.

[amareshsm]

This is looking better. Some points:

1. The value in the accordion heading should be the value of node.type is present. So "0. Object" should be "0.Member" in the following screenshot.
2. Similarly, when collapsed, we should use node.type when present. So "name Object" should be "name String".
   

2. Similarly, when collapsed, we should use node.type when present. So "name Object" should be "name String".

I’m not entirely clear on the second point. Could you clarify it for me?

Could you please review the before and after images?

Before	After

[nzakas]

@amareshsm The text displayed next to name should match the heading in the accordion, like this:

So name String because the object has a type of String.

[amareshsm]

@amareshsm The text displayed next to name should match the heading in the accordion, like this:

So name String because the object has a type of String.

Could you take a look at the "after image" in this comment and confirm if it aligns with your expectations?

[nzakas]

@amareshsm it doesn't. The value next to name doesn't match the value in the head of the accordion.

Also, your before/after is of the Scope tab but the AST tab is a better example.

[harish-sethuraman]

@nzakas Im confused on what to match with what. Consider the example here:

near source we should be showing Literal instead of Node? if that is the case accordion heading is also same even before triggering the + sign?

It would look like this if Im understanding correctly. We should change these only for objects? Because array can have different types right?

[nzakas]

It would look like this if Im understanding correctly. We should change these only for objects? Because array can have different types right?

Yes, that's correct. Anytime an object has a type property, that is the value that should be displayed everywhere.

[harish-sethuraman]

fixed. Please confirm if the changes are behaving as expected.

[nzakas]

@harish-sethuraman this looks good in the AST pane. 👍

For some reason, the scope pane is completely broken for me:

[harish-sethuraman]

@nzakas can you share the code thats in the left pane? and also which mode you were in?

[harish-sethuraman]

We have made the following changes in the PR:

• The title of the accordion is now the type that is present inside
• The subtext that is shown near expandable menu is also the type of the element present inside
• when there is only one item it wont show 0.
• added a path prop to the elements so that accordion collapses dont bug out
• now the + symbol will not be visible when the object is empty / array is empty. It will show only for those elements with value

[nzakas]

@harish-sethuraman it doesn't matter what the code is, I see this error 100% of the time. Any JavaScript triggers it for me. But it also appears that this is occuring on main, so we don't need to address it here.

[nzakas]

LGTM. Would like @amareshsm to review before merging.

[amareshsm]

@harish-sethuraman it doesn't matter what the code is, I see this error 100% of the time. Any JavaScript triggers it for me. But it also appears that this is occuring on main, so we don't need to address it here.

It is related to this issue I guess eslint/js#625

[nzakas]

Ah good call!

[amareshsm]

LGTM