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

[full-ci] Support audio formats in MediaViewer App & rename it to Preview #6514

Merged
merged 12 commits into from
Apr 6, 2022
Merged

[full-ci] Support audio formats in MediaViewer App & rename it to Preview #6514

merged 12 commits into from
Apr 6, 2022

Conversation

kulmann
Copy link
Contributor

@kulmann kulmann commented Mar 3, 2022 •
edited
Loading

Description

Renaming the mediaviewer app to preview and adding support for audio playback.
Also added support for overwriting the label of an editor's "Open with" action so that we can display Preview instead of Open in Preview.
While renaming the mediaviewer to preview is a breaking change, I've added alias handling with deprecation warnings so that configuration changes can happen over time.

Related Issue

Motivation and Context

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Screenshots (if appropriate):

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

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • rename mediaviewer to Preview
  • file action name should not be Open in Preview but only Preview
  • revert to hardcoded formats for audio, video, image (but revisit supported formats, especially for audio and don't use file extensions anymore but mimetypes instead)
  • icon for Preview -> eye
  • check if we can change pdf viewer icon to the file type icon

@update-docs
Copy link

update-docs bot commented Mar 3, 2022

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@pascalwengerter pascalwengerter mentioned this pull request Mar 3, 2022
Closed
10 tasks
@ownclouders
Copy link
Contributor

Results for oC10Files4 https://drone.owncloud.com/owncloud/web/23221/19/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

@dschmidt
Copy link
Member

dschmidt commented Mar 3, 2022

Does this fix #5273?

@kulmann kulmann mentioned this pull request Apr 1, 2022
Merged
@kulmann kulmann force-pushed the media-viewer-improvements branch 2 times, most recently from fc0c824 to 6e4d9a7 Compare April 4, 2022 13:48
@ownclouders
Copy link
Contributor

ownclouders commented Apr 4, 2022 •
edited
Loading

Results for oCISFiles2 https://drone.owncloud.com/owncloud/web/24530/56/1

💥 The acceptance tests failed on retry. Please find the screenshots inside ...

webUIFilesActionMenu-fileFolderActionMenu_feature-L14.png

webUIFilesActionMenu-fileFolderActionMenu_feature-L14.png

kulmann added 7 commits April 5, 2022 13:35
@kulmann
Allow all image, video and audio mime types for media viewer
c20282e
@kulmann
Cleanup variables and fix displayed filename on error
a744eb4
@kulmann
Refactor fetching medium into own method
9bc731c
@kulmann
Rename media-viewer to preview
42697e5
@kulmann
Revert back to a handpicked list of mimetypes
dadc77c 
Using mimetype categories turned out to be a bad idea as of now because
we would need to put a lot of thought into usability edge cases (like
falling back to raw images if they are small enough, ignoring files that
are unsupported in browsers, ...)
@kulmann
Add optional fill type for actions in context menu
4727134
@kulmann
Allow editors to define a label per extension/mimeType
ddd54b7 
For the preview app we don't want `Open in Preview` as a label for
actions, but instead only `Preview` because that's already self
explanatory. Introduced an optional `label` prop in app manifest per
extension/mimeTye and used it for the preview app.
@kulmann kulmann force-pushed the media-viewer-improvements branch from 6e4d9a7 to 88a03ec Compare April 5, 2022 11:35
@kulmann kulmann changed the title WIP support more audio, video and image formats in media viewer support more audio, video and image formats in media viewer Apr 5, 2022
@kulmann kulmann marked this pull request as ready for review April 5, 2022 11:37
@kulmann kulmann changed the title support more audio, video and image formats in media viewer support audio formats in media viewer in rename media viewer to preview Apr 5, 2022
@kulmann
Copy link
Contributor Author

kulmann commented Apr 6, 2022

As noticed by @pascalwengerter opening preview for a file in a project-space or in a share now fails and complains that the file.mimeType property doesn't exist. Checking that next..

@pascalwengerter pascalwengerter changed the title support audio formats in media viewer in rename media viewer to preview Support audio formats in MediaViewer App && rename it to Preview Apr 6, 2022
@kulmann
Copy link
Contributor Author

kulmann commented Apr 6, 2022 •
edited
Loading

As noticed by @pascalwengerter opening preview for a file in a project-space or in a share now fails and complains that the file.mimeType property doesn't exist. Checking that next..

Turns out that this was not related to the file context but only happened if the current folder had a subfolder in it. Folders have undefined as mimetype. Fixed in 73880ac
cc @pascalwengerter

pascalwengerter
pascalwengerter approved these changes Apr 6, 2022
View reviewed changes
Copy link
Contributor

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

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

Works like a charm!

changelog/unreleased/change-rename-mediaviewer Outdated Show resolved Hide resolved
@kulmann kulmann force-pushed the media-viewer-improvements branch from 848795b to 73880ac Compare April 6, 2022 12:11
@kulmann kulmann changed the title Support audio formats in MediaViewer App && rename it to Preview [full-ci] Support audio formats in MediaViewer App & rename it to Preview Apr 6, 2022
@kulmann
Fix acceptance tests & re-rename download trigger function
82740a8 
triggerDownload had a name clash with a custom global mixin.
Also re-introduce webm support via preview.
@kulmann kulmann force-pushed the media-viewer-improvements branch from 1a337c0 to 82740a8 Compare April 6, 2022 14:49
@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 6, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

3.9% 3.9% Coverage
0.0% 0.0% Duplication

@kulmann kulmann merged commit a41fdc8 into master Apr 6, 2022
@delete-merged-branch delete-merged-branch bot deleted the media-viewer-improvements branch April 6, 2022 16:00
ownclouders pushed a commit that referenced this pull request Apr 6, 2022
@kulmann
commit a41fdc8
9ef99e8 
Merge: 4ad8199 82740a8
Author: Benedikt Kulmann <[email protected]>
Date:   Wed Apr 6 18:00:41 2022 +0200

    Merge pull request #6514 from owncloud/media-viewer-improvements
@pascalwengerter pascalwengerter mentioned this pull request May 5, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pascalwengerter pascalwengerter pascalwengerter approved these changes

@fschade fschade Awaiting requested review from fschade

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kulmann @ownclouders @dschmidt @pascalwengerter