Skip to content

Fix Crash on Related Items Enqueue Popup#1

Merged
james-rensch merged 2 commits intodevfrom
fix-related-items-enqueue-on-video-change
Oct 17, 2024
Merged

Fix Crash on Related Items Enqueue Popup#1
james-rensch merged 2 commits intodevfrom
fix-related-items-enqueue-on-video-change

Conversation

@james-rensch
Copy link
Owner

@james-rensch james-rensch commented Oct 16, 2024

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • Changes the InfoItemDialog popup modal for the RelatedItemsFragment to be attached to the parent context of the RelatedItemsFragment. This prevents the popup not being attached a context when the player automatically changes to the next video and the RelatedItemsFragment is re-initialized.

Fixes the following issue(s)

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR. You can find more info and a video demonstration on this wiki page.

Due diligence

@james-rensch james-rensch self-assigned this Oct 16, 2024
@jiahengguo123
Copy link

This PR looks good to me. Developer had considered a variety of scenarios which is crucial for a robust fix. This approach of managing the InfoItemDialog context within RelatedItemsFragment is considered and effectively solves the issue.

@jiahengguo123
Copy link

This PR looks good to me. Developer had considered a variety of scenarios which is crucial for a robust fix. This approach of managing the InfoItemDialog context within RelatedItemsFragment is considered and effectively solves the issue.

I would suggest further improvements to the documentation in the code. Adding some comments explaining the need to check the context of the parent fragment can greatly help the future maintenance. Specifically, it would be helpful to detailed on how this check relates to preventing context loss during video conversion, which is essential for resolving the reported crash.

@Gc0rp
Copy link

Gc0rp commented Oct 17, 2024

Looks good to me, lovely work.

@james-rensch james-rensch changed the title Fix Crash on Related Items Modal Fix Crash on Related Items Enqueue Popup Oct 17, 2024
@james-rensch james-rensch merged commit 678f0a7 into dev Oct 17, 2024
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.

3 participants