Unify & @Stateful ItemSubtitlesViewModel#1944
Open
JPKribs wants to merge 11 commits intojellyfin:mainfrom
Open
Unify & @Stateful ItemSubtitlesViewModel#1944JPKribs wants to merge 11 commits intojellyfin:mainfrom
ItemSubtitlesViewModel#1944JPKribs wants to merge 11 commits intojellyfin:mainfrom
Conversation
Member
Author
|
I'm away from my computer but I will likely be changing this this evening to mirror how the search/denounce works from: #1945 I think the way that I did it here is incorrect and I should better reflect how it's done in item identification |
Member
Author
Okay, I believe this is resolved now! Please let me know if that is not the case! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates
SubtitleEditorViewModelto be calledItemSubtitlesViewModeland make it@Stateful. Update all impacted views to work using the new@StatefulAPIs. Finally, cleanup the subtitle search view to utilized the shared iOS & tvOS form overload since this is used in both environements. All other views are just cleaned up but otherwise still just iOS since we cannot upload or edit subtitles from tvOS only search.On top of these core changes, I've done some more generic cleanup:
StateAdapterwhere possible. OnItemSubtitlesViewI couldn't figure out how to use this for a shared delete button since it seems to freak out when used in a Swipe Action so that's the only instances I left with a state isPresenting bool.ItemSubtitlesViewwhenFirstAppearsince we need that for all the subtitles in case of change.EditModeinsread of@State isEditing. It's my understanding that when a view owns the edit state it should useEditMode. If the view should be able to observe the editing state but NOT be able to change it, then we use isEditing. Let me know if that is incorrect!ViewModelto combine some functions since they're just adding clutter and only used in a single place.ViewModelfor things like language selection since that will be used for upload as well. This cleans up the need to init that per view and preserved it between upload and search.ViewModel. Primarily, this is because the view had a try catch in it and that meant I needed an error variable and track theViewModel.error so getting this all in theViewModelmade that cleaner. Also builds the upload dto in theViewModelto trim down the view itself.Issues
Not an issue but I added:
Because "" /
Nonedoesn't work for our subtitle search API. I have a catch to just return [] but it would be cleaner IMO to just be able to removeNoneas an option. I was thinking like, if the binding isString?we includeNoneotherwise we don't ifString.IMO, an issue for another time but I left a TODO.
So, when I first did this I had a like 1.5 second wait since it seems to take Jellyfin roughly that long to show the subtitles on upload/deletion. Right now, we don't have this, instead relying on the item update to show our changes. This can result in an issue where you upload or search & set a subtitle and the list doesn't update. I'm not sure why this is the case. I believe this is more on the server that the server should have the item fully updated prior to marking the task as complete?
Some solutions I thought of (none of these are ideal):
itemwhennewItem!=itemthen we can ensure that we're capturing the change.onFirstAppearto.onAppearand refresh the viewModel on the view appearance. This is kind of better since this let's the user make multiple subtitle changes and we're not updating the item until it would actually matter in the view itself.Why can't we just make the update in the local file?
We perform deletions from the subtitles based on the index. As we know, Jellyfin subtitle indexes can surprise us. By re-getting the subtltes, we know the indexes are correct and prevent deletions of the wrong index.