Skip to content

Add as viewer plugin#10

Merged
v1r0x merged 15 commits intomasterfrom
viewer
Jul 1, 2020
Merged

Add as viewer plugin#10
v1r0x merged 15 commits intomasterfrom
viewer

Conversation

@v1r0x
Copy link
Copy Markdown
Owner

@v1r0x v1r0x commented Apr 15, 2020

Fix #4 #5 #7
Ref #1

Comment thread .eslintrc.js
@@ -0,0 +1,96 @@
module.exports = {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Comment thread appinfo/app.php Outdated
OCP\Util::addScript('files_3d', 'loader');
$eventDispatcher = \OC::$server->getEventDispatcher();
$eventDispatcher->addListener(
'OCA\Files::loadAdditionalScripts',
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused. I don't have (and don't need, I think?) any controllers. I tried to copy the appinfo setup from the viewer app itself, but that doesn't work. Any ideas 😊 ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because you don't have a dedicated page view :)
The mechanism is the same,

You just need to listen to the Viewer event, the files app is firing it
use OCA\Viewer\Event\LoadViewer;

https://github.com/nextcloud/viewer/blob/master/lib/Listener/LoadViewerScript.php
https://github.com/nextcloud/viewer/blob/b16ba3ab0dc81a9e98a88a9917936c527e94cca6/lib/AppInfo/Application.php#L42

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleanup your app.php so it query your main Application.php
https://github.com/nextcloud/viewer/blob/master/appinfo/app.php

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied the (imo 😅 ) interesting parts from app.php and Application.php from viewer to my app, but that didn't work. I'll push my latest changes tomorrow :)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, pûsh and i'll have a look :)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed my changes :)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented!

Comment thread appinfo/info.xml Outdated
<screenshot>https://github.com/ghraw/v1r0x/files_3d/master/screenshots/screenshot1.png</screenshot>
<dependencies>
<nextcloud min-version="14" max-version="16"/>
<nextcloud min-version="16" max-version="19"/>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use 17 btw as 16 is EOL in a few weeks ;)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the Viewer event might be for 18 only, so maybe your new viewer integration should come from 18 and above :)

Comment thread src/components/Files3d.vue Outdated

export default {
name: 'Files3d',
props: ['mime', 'path'],
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, thanks. Is the readme outdated? There are only path and mime mentioned

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, you theoritecally do'nt really need more than a few, but yes, I guess it needs an update, sorry abou that :)

Comment thread src/main.js Outdated
Comment thread webpack.common.js Outdated
@skjnldsv
Copy link
Copy Markdown
Collaborator

skjnldsv commented Apr 15, 2020

If you're using chrome/ium, you can use the vuejs dev tools that will allow you to see the props and components (amongst lots of things), in firefox this doesn't work because of nextcloud's strict csp policy and FF applying the same rules to the extensions :(

image

Signed-off-by: Vinzenz Rosenkranz <vinzenz.rosenkranz@posteo.de>
Comment thread appinfo/app.php Outdated
Comment thread lib/Appinfo/Application.php Outdated
Comment thread lib/Appinfo/Application.php Outdated
Comment thread .babelrc.js Outdated
v1r0x added 2 commits April 16, 2020 10:22
Signed-off-by: Vinzenz Rosenkranz <vinzenz.rosenkranz@posteo.de>
Signed-off-by: Vinzenz Rosenkranz <vinzenz.rosenkranz@posteo.de>
Comment thread lib/Appinfo/Application.php Outdated
$this->mimeTypeDetector = $mimeTypeDetector;

$server = $this->getContainer()->getServer();
$eventDispatcher = $server->query(IEventDispatcher::class);
Copy link
Copy Markdown
Collaborator

@skjnldsv skjnldsv Apr 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHould also be injected similarely as IMimeTypeDetector :)
And then move the addServiceListener to the register function I guess :)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I think I should cleanup the viewer code too) 🙈

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHould also be injected similarely as IMimeTypeDetector :)
And then move the addServiceListener to the register function I guess :)

Still doesn't work 🤔
Should the script be loaded on loading any page of my nc instance? or only when opening a supported file

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the script be loaded on loading any page of my nc instance? or only when opening a supported file

The script will be loaded on every app that request the Viewer.
So it should be loaded on files

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm looking

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using latest nc stable release and cloned my app into /apps

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docker?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, old school ;) downloaded the zip and running on local apache (in /var/www/html)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it might not work, as the zip is a stable release and contains far more than just server and viewer :)

I would suggest you checkout the server repo and clone the viewer branch in apps :)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works 🎉 I had to clone viewer as well, but now my js file is loaded 🥳

thank you very much! Now I "simply" have to render the files ;)

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Comment thread appinfo/app.php Outdated

const APP_ID = 'files_3d';

public function __construct() {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ALso, I was wrong, you cannot do dependency injection in Application.php :)
It only works after the app is initialized, so in other php classes :)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I reverted it back to how it should be

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I was confused why you changed that part :D

@v1r0x
Copy link
Copy Markdown
Owner Author

v1r0x commented Apr 16, 2020

@skjnldsv one last question :D

When trying to parse the file, I don't have a proper link to load the file. Do I have to do something like https://github.com/nextcloud/viewer/blob/b16ba3ab0dc81a9e98a88a9917936c527e94cca6/src/mixins/PreviewUrl.js#L41-L52 to get the file? If so, is there a better solution than copy&pasting all related stuff?

@skjnldsv
Copy link
Copy Markdown
Collaborator

If so, is there a better solution than copy&pasting all related stuff?

Good question!
That would be nice to improve there :/
I'm testing if we can force apply the mixin for everyone here :)

@skjnldsv
Copy link
Copy Markdown
Collaborator

Nice it works!!
Please try the viewer branch nextcloud/viewer#471

Signed-off-by: Vinzenz Rosenkranz <vinzenz.rosenkranz@posteo.de>
@v1r0x
Copy link
Copy Markdown
Owner Author

v1r0x commented Apr 17, 2020

Thanks for the quick fix for viewer!

I have another last question 😅
The size of my container, webgl renderer and canvas is (for testing) hardcoded to 700px / 600px.
I tried to use both this.height and this.naturalHeight, but they are both undefined/null. Is there a predefined way to get the available space?

@skjnldsv
Copy link
Copy Markdown
Collaborator

Yep, naturalHeight depends on the main element
this is in the html standards, this entirely depends on what you use here :)

Signed-off-by: Vinzenz Rosenkranz <vinzenz.rosenkranz@posteo.de>
@skjnldsv
Copy link
Copy Markdown
Collaborator

The size of my container, webgl renderer and canvas is (for testing) hardcoded to 700px / 600px.

Does 3d models have a standard default viewport?
If so use those?
Otherwise you can always use the maximum with 100vh/100vw and the viewer should limit this with max/min, not sure it maks sense 🤔

@v1r0x
Copy link
Copy Markdown
Owner Author

v1r0x commented Apr 17, 2020

I totally forgot about 100vh and 100vw! Damn bootstrap :D

I thought naturalHeight is a general property I didn't know, so I got confused.
Now I've set my main element to 100vw/vh and get the values with this.$el.offsetWidth/Height
Works fine so far :)

v1r0x and others added 4 commits April 17, 2020 13:01
@v1r0x v1r0x merged commit efb904c into master Jul 1, 2020
@v1r0x v1r0x deleted the viewer branch July 1, 2020 09:12
@v1r0x
Copy link
Copy Markdown
Owner Author

v1r0x commented Jul 1, 2020

@skjnldsv Thanks for your help. Release is finally ready! 🎉

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.

Integrate into nextcloud 16 viewer

2 participants