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

Add 'Reasons to use Nextcloud in your organization' call to action in settings #22136

Merged
merged 1 commit into from
Sep 7, 2020

Conversation

jancborchardt
Copy link
Member

@jancborchardt jancborchardt commented Aug 6, 2020

As discussed @karlitschek

Need a developer to take over or offer advice on:

  • Actually making the document open in the viewer / files_pdfviewer on click of the button, cc @juliushaertl @skjnldsv
  • Decide if the "skeleton" folder is the right place to put this – it’s the only folder with documents so I figured it’s as good as any, and will also make that file show up in a fresh dev instance which isn’t bad either.

(The file is not the final version yet as Marija and I are still polishing it up.)

image

@jancborchardt jancborchardt added enhancement design Design, UI, UX, etc. 2. developing Work in progress labels Aug 6, 2020
@jancborchardt jancborchardt added this to the Nextcloud 20 milestone Aug 6, 2020
Copy link
Member

@karlitschek karlitschek left a comment

Choose a reason for hiding this comment

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

great! 👍

@MorrisJobke MorrisJobke mentioned this pull request Aug 11, 2020
57 tasks
@jospoortvliet
Copy link
Member

image

Should the icon not be white, too? This grey looks a bit off...

@jancborchardt
Copy link
Member Author

@jancborchardt yep it should, will adjust as soon as we have the final document :)

@skjnldsv
Copy link
Member

  • Actually making the document open in the viewer / files_pdfviewer on click of the button, cc @juliushaertl @skjnldsv

files_pdfviewer is still not using viewer, so no idea what their api is

@juliushaertl
Copy link
Member

files_pdfviewer is still not using viewer, so no idea what their api is

There is none most likely. Also using this will only work if the file is in the example files and not removed by the user. So I'd rather vote to use something like https://www.npmjs.com/package/vue-pdf and show it in a modal just for personal the settings.

@skjnldsv
Copy link
Member

skjnldsv commented Aug 12, 2020

So I'd rather vote to use something like npmjs.com/package/vue-pdf and show it in a modal just for personal the settings.

How hard would it be to move to viewer 🙈 ?

EDIT: let me try to see that today for 20

@jancborchardt jancborchardt force-pushed the design/reasons-to-use-nextcloud branch from e684e29 to 4cb9f25 Compare August 17, 2020 11:41
@jancborchardt
Copy link
Member Author

Added the final pdf document to core/skeleton now, will have to hand this over then – to you @skjnldsv?

@juliushaertl
Copy link
Member

juliushaertl commented Aug 17, 2020

Added the final pdf document to core/skeleton now, will have to hand this over then – to you @skjnldsv?

Should be in https://github.com/nextcloud/example-files i think. ;)

Ah my bad, missed nextcloud/example-files#10

@skjnldsv
Copy link
Member

Added the final pdf document to core/skeleton now, will have to hand this over then – to you @skjnldsv?

sure commit incoming

@skjnldsv
Copy link
Member

A CSP issue, I asked Roeland for help 👍
Otherwise implementation is done with conditional rendering of the file is not in the user folder anymore!

@rullzer
Copy link
Member

rullzer commented Aug 18, 2020

nextcloud/files_pdfviewer#201
The fix :)

if (button) {
button.addEventListener('click', function() {
OCA.Viewer.open({
path: '/Reasons to use Nextcloud.pdf',
Copy link
Member

Choose a reason for hiding this comment

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

Will it actually be in your home folder?

Copy link
Member

Choose a reason for hiding this comment

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

According to skeleton, yes, no?

@rullzer
Copy link
Member

rullzer commented Aug 18, 2020

@jancborchardt @karlitschek we allow admins to set custom skeleton files. In that case we have no way to link to the file. How would be open it then?

@karlitschek
Copy link
Member

@rullzer Agreed. I think the file needs to live somewhere else. Not in the skeleton

@jancborchardt
Copy link
Member Author

@jancborchardt @karlitschek we allow admins to set custom skeleton files. In that case we have no way to link to the file. How would be open it then?

Yes, as said I have no idea where to put it – I think it’s good to have it in example files and skeleton just to show it to devs and users. But yes, for it to properly work it also needs to live somewhere permanently → and that’s your call @rullzer.

@rullzer
Copy link
Member

rullzer commented Aug 18, 2020

So actually putting the file somewhere we can. But the issue is a bit that I think the viewer is only build to handle opening from webdav. Which we can't if it is nto in your main files...

@skjnldsv
Copy link
Member

But the issue is a bit that I think the viewer is only build to handle opening from webdav. Which we can't if it is nto in your main files...

Yep, only if it's within DAV

@rullzer
Copy link
Member

rullzer commented Aug 19, 2020

Ok so that is a bit problematic then.

@jancborchardt else we just serve the file from a controller and people open it int he pdf app of their chosing... I think almost everybody can properly open pdfs.

@skjnldsv skjnldsv force-pushed the design/reasons-to-use-nextcloud branch from 73c1ffd to bcc27ad Compare September 1, 2020 09:15
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Sep 1, 2020
@rullzer
Copy link
Member

rullzer commented Sep 1, 2020

still conflicts :S

@rullzer rullzer mentioned this pull request Sep 3, 2020
13 tasks
@rullzer rullzer force-pushed the design/reasons-to-use-nextcloud branch 2 times, most recently from c2f583c to 95f0892 Compare September 3, 2020 20:29
@rullzer
Copy link
Member

rullzer commented Sep 3, 2020

/compile amend /

@skjnldsv
Copy link
Member

skjnldsv commented Sep 4, 2020

Rebased ready to go

@skjnldsv skjnldsv force-pushed the design/reasons-to-use-nextcloud branch from 95f0892 to 651f177 Compare September 4, 2020 17:59
@skjnldsv skjnldsv added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Sep 4, 2020
@skjnldsv
Copy link
Member

skjnldsv commented Sep 4, 2020

So many failures

@rullzer rullzer force-pushed the design/reasons-to-use-nextcloud branch 3 times, most recently from d74bf5c to b25e6d1 Compare September 6, 2020 18:11
@faily-bot
Copy link

faily-bot bot commented Sep 6, 2020

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 32717: failure

mysql5.6-php7.2

Show full log
There were 2 warnings:

1) Test\Files\ViewTest::testRenameFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

2) Test\Files\ViewTest::testCopyFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

--

There was 1 failure:

1) Test\Files\ObjectStore\ObjectStoreStorageTest::testCopy with data set #1 ('/source.txt', '/target with space.txt')
Expected /target with space.txt to be a copy of /drone/src/tests/data/lorem.txt
Failed asserting that false matches expected 'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.\n
Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.\n
Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.\n
Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.'.

/drone/src/tests/lib/Files/Storage/Storage.php:222
/drone/src/tests/lib/Files/Storage/Storage.php:235

… settings

Signed-off-by: Jan C. Borchardt <hey@jancborchardt.net>
@rullzer rullzer force-pushed the design/reasons-to-use-nextcloud branch from b25e6d1 to 4c48d6b Compare September 7, 2020 06:30
@rullzer rullzer merged commit 16e1d1c into master Sep 7, 2020
@rullzer rullzer deleted the design/reasons-to-use-nextcloud branch September 7, 2020 06:49
"version": "7.10.5",
"resolved": "https://registry.npmjs.org/@babel/cli/-/cli-7.10.5.tgz",
"integrity": "sha512-j9H9qSf3kLdM0Ao3aGPbGZ73mEA9XazuupcS6cDGWuiyAcANoguhP0r2Lx32H5JGw4sSSoHG3x/mxVnHgvOoyA==",
"version": "7.11.5",
Copy link
Member

Choose a reason for hiding this comment

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

🙄

Copy link
Member

Choose a reason for hiding this comment

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

lots of packages were updated unintentionally 🐘

Copy link
Member

Choose a reason for hiding this comment

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

Sometimes done automatically :p
If it's deps of deps

Copy link
Member

Choose a reason for hiding this comment

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

Nope. Only if you npm update or npm i another dependency. But this doesn't happen just so.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

No clue!
It's merged now 🤷

Copy link
Member

Choose a reason for hiding this comment

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

Then let's Yolo on! 🎉

@MorrisJobke
Copy link
Member

@rullzer Psalm said no: #22650 😉

@jospoortvliet
Copy link
Member

@jancborchardt yep it should, will adjust as soon as we have the final document :)

So sadly this made it into the release 🙈

#24450

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress design Design, UI, UX, etc. enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants