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

Fix for Region for Trending dropdown not populating #2737

Merged

Conversation

MarmadileManteater
Copy link
Contributor

@MarmadileManteater MarmadileManteater commented Oct 17, 2022

Fix for Region for Trending dropdown not populating

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

#1644

Description

This PR aims to fix an issue with the Region for Trending dropdown not populating with data before the user changes the currentLocale.

Screenshots

Before:

After:

Testing

  1. Delete your settings.db file (I believe it is located in the Electron roaming directory when debugging)
  2. Start FreeTube
  3. Check the Region for Trending dropdown in settings

Desktop (please complete the following information):

  • OS: Windows 10
  • OS Version: Pro Version 21H2 Installed on ‎4/‎3/‎2022 OS build 19044.1889 Experience Windows Feature Experience Pack 120.2212.4180.0
  • FreeTube version: 0.17.1

Additional Information

An alternative solution might be to add an additional call to getRegionData somewhere; however, it may be difficult to tell if getRegionData has already been called recently without introducing a race condition. That could cause getRegionData to be called twice in a row. The reason I went with this solution was because this issue goes away when all of the side effects are evaluated on app mounted.

Previously, only settings which have a user defined value would
be iterated on in this loop. Now, all settings that have a user defined
value or that have side effects will be iterated. This is important because
`currentLocale`'s side effect handler contains a call to `getRegionData`,
and if this side effect handler is never called, the `Region for Trending`
option in settings will never populate.
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Oct 17, 2022
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) October 17, 2022 16:22
Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Tested locally

@FreeTubeBot FreeTubeBot merged commit c26445a into FreeTubeApp:development Oct 18, 2022
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Oct 18, 2022
@MarmadileManteater MarmadileManteater deleted the empty-settings-bug branch March 16, 2024 15:55
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.

5 participants