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.

Sign up for GitHub

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

Jump to bottom

Mediaviewer app #1530

Merged
merged 1 commit into from
Jul 23, 2019
Merged

Mediaviewer app #1530

merged 1 commit into from
Jul 23, 2019

Conversation

felixheidecke
Copy link
Contributor

@felixheidecke felixheidecke commented Jul 16, 2019 •
edited
Loading

Description

Adding media viewing capabilities. All still work in progress

Screenshots (if appropriate):

2019-07-19 16-30-36 2019-07-19 16_32_09

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Open tasks:

  • Return to referrer on close
  • Show image name
  • Add download button

@felixheidecke felixheidecke self-assigned this Jul 16, 2019
@ownclouders
Copy link
Contributor

💥 Acceptance tests webUIFiles failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/4121/3

20190716-071503-910.png
20190716-071731-271.png
20190716-071802-451.png
20190716-071833-166.png
20190716-071912-580.png
20190716-072024-042.png
20190716-072054-805.png
20190716-072127-458.png
20190716-072158-259.png
20190716-072333-298.png
20190716-072412-409.png

DeepDiver1975
DeepDiver1975 requested changes Jul 16, 2019
View reviewed changes
apps/mediaviewer/src/Mediaviewer.vue Outdated Show resolved Hide resolved
apps/mediaviewer/src/Mediaviewer.vue Outdated Show resolved Hide resolved
apps/mediaviewer/src/PdfViewerAppBar.vue Outdated Show resolved Hide resolved
apps/mediaviewer/src/app.js Outdated
@@ -0,0 +1,28 @@
import '@babel/polyfill'
// import translationsJson from '../l10n/translations'
Copy link
Member

Choose a reason for hiding this comment

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

we need to bring this back - I'll take care in follow up PRs

@DeepDiver1975 DeepDiver1975 mentioned this pull request Jul 16, 2019
Closed
2 tasks
@felixheidecke felixheidecke mentioned this pull request Jul 16, 2019
Merged
DeepDiver1975
DeepDiver1975 requested changes Jul 17, 2019
View reviewed changes
Copy link
Member

@DeepDiver1975 DeepDiver1975 left a comment

Choose a reason for hiding this comment

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

  • rebase
  • sqaush to reasonable unit

DeepDiver1975
DeepDiver1975 requested changes Jul 17, 2019
View reviewed changes
apps/mediaviewer/src/Mediaviewer.vue Outdated
}

// Set initial file
_.filter(this.mediaFiles, (file, index) => {
Copy link
Member

Choose a reason for hiding this comment

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

dont use underscore/lodash - use https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/filter

apps/mediaviewer/src/Mediaviewer.vue Outdated
_.filter(this.mediaFiles, (file, index) => {
if (file.path === this.$store.getters.activeFile.path) {
this.activeIndex = index
console.log(index)
Copy link
Member

Choose a reason for hiding this comment

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

remove please

apps/mediaviewer/src/Mediaviewer.vue Outdated
...mapGetters('Files', ['activeFiles']),
mediaFiles () {
return _.filter(this.activeFiles, file => {
return file.extension.toLowerCase().match(/(png|jpg|jpeg)/)
Copy link
Member

Choose a reason for hiding this comment

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

no gif? no raw image?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added gif, but my OC doesn't support any raw files :-/

package.json Outdated
@@ -111,5 +111,8 @@
"eslint --fix",
"git add"
]
},
"dependencies": {
"query-string": "^6.8.1"
Copy link
Member

Choose a reason for hiding this comment

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

dev dependency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

apps/mediaviewer/src/Mediaviewer.vue Outdated Show resolved Hide resolved
@DeepDiver1975
Copy link
Member

and please use the folder name 'media-viewer' THX

@DeepDiver1975 DeepDiver1975 force-pushed the app/mediaviewer branch 2 times, most recently from 51c54e2 to 954f5db Compare July 17, 2019 13:28
@ownclouders
Copy link
Contributor

💥 Acceptance tests webUILogin failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/4164/4

20190717-141119-912.png
20190717-141136-866.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests webUISharingInternalUsers failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/4163/9

20190717-141217-739.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests webUISharingInternalUsers failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/4167/9

20190717-180645-650.png
20190717-180645-951.png
20190717-180646-251.png
20190717-180646-554.png
20190717-180646-805.png
20190717-180647-024.png
20190717-180734-674.png
20190717-180735-094.png
20190717-180735-419.png
20190717-180735-667.png
20190717-180736-032.png
20190717-180736-367.png
20190717-180804-056.png

@felixheidecke
Copy link
Contributor Author

Will squash if all is good.

@felixheidecke
Copy link
Contributor Author

Whilst working on the media viewer, I found out that data blobs don't get cached, so images may get fetched multiple times. I guess we could use the browsers local storage to do that, so I don't have to pollute the store with big data.

What do you think?

@ownclouders
Copy link
Contributor

💥 Acceptance tests webUISharingInternalUsers failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/4168/9

20190717-182133-128.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/4168/13

20190717-182651-325.png
20190717-182651-424.png
20190717-182651-530.png
20190717-182651-684.png
20190717-182651-787.png
20190717-182651-863.png
20190717-182735-029.png
20190717-182735-123.png
20190717-182735-228.png
20190717-182735-326.png
20190717-182735-419.png
20190717-182735-604.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests webUISharingInternalUsers failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/4197/9

20190718-075547-667.png
20190718-075548-025.png
20190718-075548-268.png
20190718-075548-494.png
20190718-075548-743.png
20190718-075549-002.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests webUISharingInternalUsers failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/4258/9

20190719-135310-852.png
20190719-135311-111.png
20190719-135311-408.png
20190719-135311-670.png
20190719-135312-007.png
20190719-135312-229.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/4258/12

20190719-140149-579.png
20190719-140149-739.png
20190719-140149-996.png
20190719-140150-283.png
20190719-140150-463.png
20190719-140150-649.png
20190719-140237-695.png
20190719-140237-939.png
20190719-140238-110.png
20190719-140238-281.png
20190719-140238-463.png
20190719-140238-622.png

@DeepDiver1975
Copy link
Member

Squash please. As usual

DeepDiver1975
DeepDiver1975 approved these changes Jul 22, 2019
View reviewed changes
apps/mediaviewer/src/app.js Outdated
extensions: [{
extension: 'png'
}, {
extension: 'jpg'
Copy link
Member

Choose a reason for hiding this comment

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

jpeg? gif?

DeepDiver1975
DeepDiver1975 reviewed Jul 22, 2019
View reviewed changes
apps/media-viewer/src/Mediaviewer.vue Outdated
}

// Set initial file
this.mediaFiles.filter((file, index) => {
Copy link
Member

Choose a reason for hiding this comment

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

not really a filter - right? use foreach?

src/plugins/mediaSource.js
resolve(reader)
}
} else if (returnAs === 'url') {
const source = window.URL.createObjectURL(blob)
Copy link
Member

Choose a reason for hiding this comment

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

where is the corresponding delete?

@felixheidecke
Add mediaviewer app
0d3c6bb 
with basic image support
@DeepDiver1975 DeepDiver1975 added Status:Needs-Review Needs review from a maintainer and removed Status:In-Progress WIP labels Jul 23, 2019
@DeepDiver1975 DeepDiver1975 merged commit ba21594 into master Jul 23, 2019
@delete-merged-branch delete-merged-branch bot deleted the app/mediaviewer branch July 23, 2019 08:11
@felixheidecke felixheidecke mentioned this pull request Jul 23, 2019
Closed
@butonic butonic mentioned this pull request Nov 11, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DeepDiver1975 DeepDiver1975 DeepDiver1975 approved these changes

@LukasHirt LukasHirt Awaiting requested review from LukasHirt

Assignees

@felixheidecke felixheidecke

Labels
Status:Needs-Review Needs review from a maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@felixheidecke @ownclouders @DeepDiver1975