Issue#111 Change behavior of second level entities

[ihordubas1]

No description provided.

[netlify]

✅ Deploy Preview for 3dstreet-editor-builds ready!

Name	Link
🔨 Latest commit	b5e4dd5
🔍 Latest deploy log	https://app.netlify.com/sites/3dstreet-editor-builds/deploys/63e43fac8eb21e00078e5c6e
😎 Deploy Preview	https://deploy-preview-117--3dstreet-editor-builds.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 settings.

[kfarr]

Hi @ihordubas1 thank you for this PR. It seems like a good start although I was unable to see what is different in the application based on this code change.

I would encourage you to see the work in progress here: #112. I'm not a great react dev so it's probably not implemented correctly, and my CSS changes broke some things, but it shows the example of having all of the entities listed in the scene graph which is part of the goal of #111

If it helps I can provide more guidance, for example having a better test case and defined output

[ihordubas1]

Hi @kfarr, thanks for provided example from your PR, I checked it.
Also I added new commit, where I added some entities for tests (we will remove it later) and added scroll.
If I understood properly, the goal of ticket was to show expanded children entities of first level items ("3D Street Layers" "2D Street Layers" "Viewer") and don't show children of second level (f.e. "3D Street Layers" --> "16th Street West of SVN") by default.
Please let me know if something isn't ok.

[kfarr]

Hi @ihordubas1 great progress, almost there! Final change:

• We want to show all entities, not just those with data-layer-name component. (In other words, remove filtering to only show entities with data-layer-name html attribute.) We may reverse this decision eventually, but for now it helps for debugging as we work on more advanced things. Also, we still want to use data-layer-name value for displaying in scene graph and property panel if it exists, we just don't want to filter by it.
• In the future we may want to make filtering by data-layer-name a user preference, or use different logic to only show entities that relevant for end-users.

[kfarr]

Actually there are a few more issues.

1. When scrolling down, we want the Layers panel heading and collapse button to remain at the top of the panel, not scroll with the scene-graph content. layers panel heading marked by red square:
   

2. Also when I click the collapse button on the left panel, the expand button no longer appears after the flex css changes. (Fixing the above issue might also fix this)

3. Finally, there is also an error if I click on the collapse button a few times, see screen cast including console error

Screen.Recording.2023-02-06.at.4.47.57.PM.mov

[ihordubas1]

Hi @kfarr, I pushed new commit where I:

• removed test entities from index.html
• changed order of layers with entities like you wrote in the Show all entities in scene graph #112 comment
• removed filtering of entities by data-layer-name html attribute
• made Layers panel heading fixed while scrolling down.

1. Concerning your 2 and 3 point about Collapse button: should this button still appear when we click on it? (like in attached screenshot below.

2. Also, when I removed filtering of entities by data-layer-name html attribute I get some another elements in layers panel, is it okay?

[kfarr]

getting closer, here are remaining issues:
(0) general concern that we are over complicating this, see native a-frame inspector (remove line 57 in index.html). We should not be adding much code in this PR, these commits should mostly be removing js code to revert to previous default behavior of a-frame inspector, and modifying styles. For example, the string data-layer-show-children should not be in any js file because we no longer respect it. [see video0]
(1) restore existing behavior of layers panel show / collapse behavior, see 3dstreet.app [see video1]
(2) include all child entities in scene graph, just don't have all parents expanded by default the panel open
(3) fatal error from texture modal - this can be fixed as part of separate ticket: #118

Here are video recordings to explain, sorry the audio is not great I'm at a cafe:

• video0: https://user-images.githubusercontent.com/470477/217402065-4eef9cd5-ba2f-4975-9672-c2354ba39c5a.mov
• video1: https://user-images.githubusercontent.com/470477/217402282-ed6e5156-c927-433b-877a-d09869fe5d04.mov

Also, when I removed filtering of entities by data-layer-name html attribute I get some another elements in layers panel, is it okay?

Yes that is ok and expected behavior

Concerning your 2 and 3 point about Collapse button: should this button still appear when we click on it? (like in attached screenshot below.

No, this is adding new behavior not expected. See video1 for explanation of restoring existing expected behavior.

[ihordubas1]

Hi @kfarr
Thanks for the videos! More clear for now.
I removed using data-layer-show-children attribute from code, made behavior of layers panel similar to native a-frame inspector, fixed issue from video1.

Just one little question from video0. You said that on 00:41 that ''expected behavior - this layer to be expanded by default''. Do you mean only 3D Street Layers to be expended or all other too? I mean 3D Street Layers, 2D Street Layers, Environment etc..

[kfarr]

this looks and works great, thank you @ihordubas1 !