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

New TV UI bug fixes #983

Merged
merged 2 commits into from
Mar 16, 2024
Merged

New TV UI bug fixes #983

merged 2 commits into from
Mar 16, 2024

Conversation

KingLucius
Copy link
Contributor

@KingLucius KingLucius commented Mar 11, 2024

  • Episode down to description closes Episodes Holder.
  • Rework main buttons visibility, should fix the visibility bugs.
  • Fix Episode space bug (Episode1)

@KingLucius
Copy link
Contributor Author

KingLucius commented Mar 11, 2024

@Luna712 Can you please support testing these fixes? Thanks

@Luna712
Copy link
Contributor

Luna712 commented Mar 11, 2024

This does fix the button visibility issues for me but I'm not sure how to test the others as I personally couldn't seem to reproduce.

@KillerDogeEmpire
Copy link
Contributor

KillerDogeEmpire commented Mar 11, 2024

@KingLucius The fixes are great, they all work good.

@KingLucius
Copy link
Contributor Author

Thanks guys for testing 🙏

@@ -646,15 +646,14 @@ class ResultFragmentTv : Fragment() {
}

observeNullable(viewModel.movie) { data ->
if (data == null) return@observeNullable
if (data == null || resumeMode) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

observe does not have a determined order, so I dont like this. It just sounds like you are making the bug less likely

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's why i am checking the resume this way, i didn't find another way to do this.
I am open for suggestions though

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would move all the visibility logic into resumeWatching and make this function act as a setOnClickListener, then do the nextFocusRightId in the loadpage section. So this makes it so:

Page fills all the metadata about the thing
Movie adds the listener
Resume watching handles the visibility

But if this is too much the viewmodel should be simplified, as all the parts should be independent and support out of order eval

Copy link
Collaborator

Choose a reason for hiding this comment

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

What you can do is making a dummy header view, that way you can have an "X AND Y" visibility, this is very common in cs3 for these things

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would move all the visibility logic into resumeWatching and make this function act as a setOnClickListener, then do the nextFocusRightId in the loadpage section. So this makes it so:

Page fills all the metadata about the thing
Movie adds the listener
Resume watching handles the visibility

But if this is too much the viewmodel should be simplified, as all the parts should be independent and support out of order eval

I got that, will rework based on that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What you can do is making a dummy header view, that way you can have an "X AND Y" visibility, this is very common in cs3 for these things

I don't understand that bro 😂 please elaborate or guide me to a code in CS i can read to get the idea as an example

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand that bro 😂 please elaborate or guide me to a code in CS i can read to get the idea as an example

Well the idea is to have a view that holds many items, see for example apply_btt_holder

android:id="@+id/apply_btt_holder"

Then you can change the visibility of the holder

But the neat thing is that it has a separate visibility from the view itself. So in this case if I wanted to toggle the visibility of apply by two bools, lets say realShowApply AND showApply then I can set the headers visibility to realShowApply and the apply buttons visibility to showApply. That way both the buttons (cancel and apply) are turned on if realShowApply is true, but the apply buttons is only visible if both realShowApply is true and showApply is true.

In your case I would group all buttons visible in one mode in one header view and turn that view on or off, but this will still keep the visibility state of each individual button.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But you can also just make a state machine, that specifies the conditions for every button given a set of paramaters, like isMovie, hasEpisodes, hasPreview, hasTrailer and update every buttons when you update this state

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I handeled most of the visibility in ResumeWatching, and in Movie I check for Resume button visibility to toggle Play button visibility

@fire-light42
Copy link
Collaborator

I was unable to replicate the bug, @Luna712 can you check?

@Luna712
Copy link
Contributor

Luna712 commented Mar 15, 2024

Sure, I can test this in a couple hours

Copy link
Contributor

@Luna712 Luna712 left a comment

Choose a reason for hiding this comment

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

This fixes all of the issues I could reproduce

@fire-light42 fire-light42 merged commit 638cc4f into recloudstream:master Mar 16, 2024
2 checks passed
@KingLucius KingLucius deleted the uiFixes branch March 16, 2024 03:20
@KingLucius KingLucius mentioned this pull request Mar 16, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants