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 some UI issues #638

Merged
merged 6 commits into from
Oct 24, 2022
Merged

Conversation

vincentfretin
Copy link
Contributor

Needs to be rebased after #637 is merged, the branch won't work if you're trying to test it

@dmarcos
Copy link
Member

dmarcos commented Oct 4, 2022

I think it doesn't need rebase. does it?

@vincentfretin
Copy link
Contributor Author

If you want to merge it right away, yeah it doesn't need a rebase.

@vincentfretin
Copy link
Contributor Author

I rebased in case you want to test the branch first before merging.

@vincentfretin
Copy link
Contributor Author

Once this is merged, please run npm run dist and commit the build so we can use a jsdelivr gh url to on the aframe inspector component.

@vincentfretin
Copy link
Contributor Author

I have a fix-some-ui-issues-with-dist branch that is based on this PR where I committed the dist. There is also a commit to fix some style issues when the inspector open on a page that includes bootstrap or tailwindcss reset styles. I'll do a separate PR for this, there are still 2-3 things that needs to be fixed.
You can test in your project with

<a-scene inspector="url:https://cdn.jsdelivr.net/gh/vincentfretin/aframe-inspector@a6ab6b985138c12a9094a13cf47fb6173e18a737/dist/aframe-inspector.js">

@vincentfretin
Copy link
Contributor Author

vincentfretin commented Oct 5, 2022

I'm currently testing with aframe master threejs r144 with a tailwindcss theme. The latest commit I did fixes the error update method undefined because I had an avatar with SkinnedMesh in my scene.

@vincentfretin
Copy link
Contributor Author

I did some cleanup in the files in the lib folder because I'm reusing all those files but with my own ui in a project, so I went ahead and contributed it here too. I think you can merge now, those are really small changes.

Are you interested in more heavy updates, update the webpack stack, run prettier on the files again, remove the eslint rules concerning the style, integrate husky to run prettier on the files, update to latest react and api, use only import/export everywhere and not a mix of import and require. Will you merge it?

@vincentfretin
Copy link
Contributor Author

I moved the code cleanup I'm currently doing in #639 so you can concentrate on the functional issues I fixed in this PR. Do you have any comment on the things I fixed here?

src/index.js Outdated Show resolved Hide resolved
@vincentfretin
Copy link
Contributor Author

Another UI issue fixed in #648

@vincentfretin
Copy link
Contributor Author

For the animation loop fix. Before you had a NaN in the panel for the loop property when clicking on the red cube, see:
Capture d’écran du 2022-10-10 15-33-36
With the fix, you have properly a checkbox and you can disable the animation loop.

@vincentfretin
Copy link
Contributor Author

For the "entityclone" fix. It fixes the left panel not refreshing when you click on the Clone entity icon here:
Capture d’écran du 2022-10-10 15-36-48

@vincentfretin
Copy link
Contributor Author

The event.preventDefault(); fixes are to prevent panel to collapse/expand, this prevents the default behavior that is following the defined href. So before the url address bar would change to add a "#" and wrongly collapse the panel. After the fix, it just copy to clipboard, that's all.

@vincentfretin
Copy link
Contributor Author

I fixed another issue in lib/viewport.js with helper update error while trying to expand an entity on the left side panel.
image

@dmarcos
Copy link
Member

dmarcos commented Oct 24, 2022

Thank you!

@dmarcos dmarcos merged commit 196416b into aframevr:master Oct 24, 2022
@vincentfretin vincentfretin deleted the fix-some-ui-issues branch December 6, 2022 16:16
vincentfretin added a commit to vincentfretin/aframe-inspector that referenced this pull request Dec 24, 2022
vincentfretin added a commit to vincentfretin/aframe-inspector that referenced this pull request Dec 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants