feat: add Media Info, Download, and Copy Stream URL to context menu

[noaione]

Add the following:

• Media Info
• Copy Stream URL
• Download/Download all

Also fix #1950

Split from the following PR: #1951

[noaione]

Trying to flatten the i18n key but what even is this

[ferferga]

@noaione It's an eslint plugin we have to avoid secrets in the codebase. I'll review it once I get into the linting suites refactor, maybe we can tune it a bit better to avoid false positives like this.

In any case, remember the rule of thumb: if the error message provides additional context to the user is good to have an special one for it. If it doesn't, just display the generic one and expect the user to open the console.

That message doesn't add any useful information, safe to delete it and problem gone.

[ferferga]

Just a heads up for everybody reading (specially @noaione): @ThibaultNocchi and me have been discussing this internally.
Since this PR is getting Tauri-specific logic for download (being the first PR that directly addresses integration with Tauri, wow thanks!), it might take some time to get this merged while the concerns in this discussion are addressed.

Perhaps we can cherry-pick and merge the other features and just hold Download in the meantime.

Any help is appreciated! 🙌💗

[noaione]

This PR right now actually don't have Tauri stuff implemented since I decide to remove it because a bit out-of-scope and it broke it somehow (stuck on splash screen)

The plan originally was to add it back later from another PR that includes native support later for:

• Clipboard (via @tauri-apps/api/clipboard)
• Download (via tauri-plugin-upload)

imo, keep the download part in here and just disable it on Tauri by checking either window.__TAURI__ or window.__TAURI_METADATA__. Just need to remember to remove it later when the microframework thingy are actually implemented.

[ferferga]

Took a while but finally I'm with this. Some comments (didn't go into much detail into the code):

• Download doesn't work for me

  • I don't like the fact that we do strict browser checking for this. At the Vue 2 migration we removed a supported-features.ts file we had that was solely about strict checks. We should always strive to find ways to directly ask the browser/device whether something is supported or not. We should know if the browser supports downloads or not directly, not "guess" (user agents are always hints and they're more and more unpopular each day) that Edge running in an Xbox can't download files. What if MSFT allows it one day? What if I'm running the client in an Echo Show that we don't have in our list? Possibilities are endless and we can't take all of them into account, unfortunately. Plus adding more and more exceptions is always going to be a maintenance burden.

    This is also why I want to switch to MediaCapabilities for profile generation ASAP. Current code is all based on web's and, as said above, is pure guesswork that doesn't work properly with modern codecs and browser updates. And it has been untouched since then, since there are so many edge cases considered that it's really difficult to tackle. I remember a lot of commits in Web ended up being trial and errors for optimizing the profile generation.

    With all this said, I'm not sure what's the best way forward into this, since your approach seems to be the most recommended one in the examples I've seen. We could try a mega.nz like approach, where we save to IndexedDB, then flush to a file and then remove from DB, but I'm not sure how to start with that (only thing I know it needs to be in a WebWorker and it could be a task in TaskManager!). If the "flush" to file step fails, we can assume the browser doesn't support downloading (although that might not be the best UX). There's also the FileSystem API, not supported by Safari but imo it's still so widely considered that we can consider it

    All of this we're talking about raw file download: of course a "save offline" feature would have some implementations differences.

  EDIT: This seems a good alternative? https://vueuse.org/core/useFileSystemAccess/#usefilesystemaccess

• As with the Identify PRs, you did a great work at mimicking current Web interface. But we should attempt to be more creative in that regard. Some ideas that came to my mind:

  • Close button with i-mdi-close like I did with Identify instead of one with text below (we should create a GenericDialog component, since we're basically duplicating a lot of stuff this way
  • Streams could be tabs inside the MediaInfo dialog. Something that follows Type - Name (Type if no name) for instance?
  • Instead of "Yes" or "No" (which are not translated by the way), we could use disabled checkboxes? https://vuetifyjs.com/en/components/checkboxes/

• In general, I don't understand why so many components? You can loop through an array of objects interfaced like this:

interface Attributes {
name: string;
value: string;
type?: 'string' | 'boolean';

Do note string and boolean are strings, not primitive types. That way you can display the checkboxes and be flexible enough to add an additional value (like number) if you want to do rounding and things like that.
Maybe aggregating all of that into a single one is too much, but you can try to divide them by media type! (Just like we do in Item menu, where we have the actions in various functions).

• Is really necessary the ability to copy to clipboard the media info? Isn't easier to simply drag my mouse with right click? In that context, it doesn't make much sense to me.

Try playing a bit with these changes and later on I can give you an extra boost if needed :). As always, don't hesitate to ping me.
@ThibaultNocchi - tagging so we have your input too!

[noaione]

Streams could be tabs inside the MediaInfo dialog. Something that follows Type - Name (Type if no name) for instance?

Could you elaborate on this? Do you mean like putting each stream type into it's own tabs like MetadataEditor?

And how should I handle multiple sources input?

Edit: Honestly, I don't like adding Name into the tab since it would be way too long. Having it as just Type {index} would be better imo.

[noaione]

we should create a GenericDialog component, since we're basically duplicating a lot of stuff this way

I did some generalization like this https://gist.github.com/noaione/818ea08274a2e03975e64fd553274eb1
I'll probably push everything after I did some of the suggestion

[ferferga]

@noaione With "multiple sources" you mean different versions of an item or the different streams a file can have? If the former, I think we should display them when that specific version is selected (I'm not sure we even have that logic implemented yet by the way). If the latter, I think we could do it in a similar way as you did: Video {{index}}, Audio {{index}}, Subtitle {{index}}

For subtitle and audio types we could add the language, I agree the stream name could be really long.

Use the normal Vuetify tabs, just like you did in your first iteration of the Identify (so them at the top, not at the side).

[noaione]

@ferferga, Currently:

Code_qPdMQilXk2.mp4

And for item with multiple source:

[ferferga]

Labeling as blocked since, thinking it more deeply, the new GenericDialog component you created would benefit a lot from the newly released Vue 3.3 features!
We can't merge those yet because Volar (vue-tsc) is still referencing the beta. There's already a PR for it upstream: vuejs/language-tools#3167

Sorry for the long text beforehand, but hopefully it helps you to learn 1 thing or 2 about Vue as you wished ☺️ (alongside the requested changes 😉).

---

Before Vue 3.3, types passed to defineProps couldn't be imported, they had to be defined locally. We discovered this with our good old TooltipButton. If you move to that point in history (you can try with Codespaces), you'll see that, although we used defineProps, we could put arbitrary props to the component and Volar would act as if they don't exist, nuking completely the point of IntelliSense (IDE Autocompletion) and type-checking.

The situation here is very similar: we have a Vuetify component we want to extend and we're manually passing props. But it's pointless to bind all of them manually (or go to the file and edit it when upstream changes the props or we want to use a non declared one!. There's also AppBarButtonLayout that cames to my mind with the same circumstances.

Check the commit I did: with it, we could simply do this:

<template>
   <v-dialog
      v-model="model"
      v-bind="props"
   </v-dialog>
   ...
  <v-card :loading="loading">
   ...
  </v-card
</template>

<script setup lang="ts">
import { PropsOf } from 'types/util';
import type { VDialog } from 'vuetify/lib/components/VDialog/index';

const props = defineProps<PropsOf<VDialog> & { loading?: boolean }>();
// Also new sugar in Vue 3.3, although I didn't investigate much about it. That's your homework :)
const model = defineModel();

**EDIT: Just checked and we might not need it. It seems Vuetify changed their definitions of components to factory functions (which I guess simplifies this?): https://github.com/vuetifyjs/vuetify/releases/tag/v3.2.3 and vuetifyjs/vuetify#17263

Here, we will have the exact same experience as using Vuetify's v-dialog (v-model as well, so we can't stop using model-value as an intermediate) but with our own component and not reinventing the wheel!

I'm in doubt though if we should do the same with v-card and have them in specific keys (in a similar way to TooltipButton). I think v-card props are less likely to be used, since it's not the main purpose of the component, but I don't have a preference honestly, so I'll 100% leave it up to you!

Although I labeled this as blocked, I invite you to create a copy of this branch locally and cherry-pick all the renovate PR's commits there, so you can work with all this goodness. Commit the changes and cherry-pick those here, so we can merge it straightaway after we merge renovate's first.

---

Now into the feedback per-se

---

MediaInfo

• I'd keep all the contents of the General tab outside of the tabs, since the contents of General are sort of relevant to all the tabs, so hierarchically I think it makes more sense. The layout would be something like:
  • Version selector
  • General contents
  • Tabs
• Instead of the language codes, I think we should use the real display names. Check @/utils/i18n. You can see examples at MediaStreamSelector component.

Download

• I've been investigating a bit and I think maybe we can check for the existence of window.open?. Scoped browsers (i.e Tauri) or Edge Xbox (which I think being 100% Chromium now has almost no limits) shouldn't have them defined? Or we can throw an specific "Your browser doesn't support downloads" message in the case they're defined but throws.

Copy stream URL

• One question that didn't come up at the beginning: what's the purpose of this? I never understood it in web when I started using Jellyfin, it gets the same URL as the Download endpoint. I see an use case on wanting to pass an item to VLC, for example. However, it isn't the same if we have a "Copy direct link" option or something like that? Or am I missing something?

• Although having our own composable might shove off a few bytes from the bundle, I prefer to use VueUse's. We can assume a better maintenance of it as it has tests, it's more widely used, has legacy support (although we shouldn't need it, but who knows...)

This way we also solve the Download vs "platforms with native" downloads (as said before, given the ES target we use, we assume all browsers have IndexedDB for offline playback, but that's not quite the same thing).

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
3 Code Smells

No Coverage information
0.0% Duplication

[noaione]

I'll try going through the review/suggestion bit by bit since my Uni started again and I've been busy with it.

[ferferga]

@noaione No rush, I'm in the same situation :)

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
3 Code Smells

No Coverage information
0.0% Duplication

[sonarcloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
1 Security Hotspot
2 Code Smells

No Coverage information
0.0% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

[jellyfin-bot]

Cloudflare Pages deployment

Latest commit	4895888
Status	✅ Deployed!
Preview URL	https://30cc6e57.jf-vue.pages.dev
Type	🔀 Preview

View build logs
View bot logs

[ferferga]

@noaione Did all the requested changes myself, except the prop generalization of the GenericDialog component. Given that we will move to Quasar, we can explore better ways to do prop type generalization better.