X-Ryl669 / grav-plugin-viewer3d

GNU Affero General Public License v3.0
2 stars 1 forks source link

Review #1

Closed pamtbaau closed 3 years ago

pamtbaau commented 3 years ago

Code has become much cleaner don't you think?

NB. Before testing, always run $ bin/grav cache to be sure you are not testing any cached data. To be save, do this after every change in code.

X-Ryl669 commented 3 years ago

Code has become much cleaner don't you think?

Indeed!

Images should now load from any location. BTW, it's not obvious how to fetch assets (like pictures, not CSS or JS) from a plugin, maybe this could be documented better in the plugin tutorial.

I've used Grav\Common\Utils::url('plugin::/viewer3d/assets/img') here, so I hope it's the correct way. Please confirm that it fixed the issue for you.

pamtbaau commented 3 years ago

Sigh... Did you test it yourself? Leftover 'show-edges` in shortcode leads to Exception...

X-Ryl669 commented 3 years ago

Not sure what you meant. It's working here. show-edges is not found in the repository. show_edges is a configuration item found in blueprint.yaml and initially in viewer3d.yaml but I've removed it as you said (not sure what is the expected role of repeating values here in different files).

In the plugin page (in the admin panel) you can enable or disable it. Enabling it toggles showing edges on the model, disabling show the model with no modifications.

X-Ryl669 commented 3 years ago

Installed from scratch here too and it works fine.

pamtbaau commented 3 years ago

You probably have a copy of the config file in /user/config/plugins/viewer3d.yaml. In my fresh install, I don't and therefor, an Exception happens in V3DShortcode.php#L18

X-Ryl669 commented 3 years ago

Ok, I've reverted the removal of show_edges in viewer3d.yaml. Now it's a boolean value and it's set to false by default. It should work better without an exception. Please let me know if it fixed it for you (does not seem to make a difference for me, but as you said, I might have cached value). I'm not finding the /user/config/plugins/viewer3d.yaml file but I had a /user/config/plugins/viewer3-d.yaml left over from previous plugin. I've removed it and it also seems to work.

pamtbaau commented 3 years ago

Please let me know if it fixed it for you

I'm afraid I've come to a point at which I feel I've done more then enough...

X-Ryl669 commented 3 years ago

That's ok, I think it's fixed now. Thanks for your time.