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

Hide sync library option when sync is disabled #1275

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

FuSan21
Copy link

@FuSan21 FuSan21 commented Sep 12, 2024

Tried to implement this draft pull (#1203) as it was inactive for a long time.

  • Add isSyncEnabled parameter to LibraryToolbar and LibraryRegularToolbar
  • Pass isSyncEnabled from LibraryTab to LibraryToolbar
  • Conditionally display sync library action based on isSyncEnabled
  • Update LibraryContent to include isSyncEnabled parameter
  • Adjust LibraryTab to handle sync-related functionality
  • Remove direct dependency on SyncPreferences in LibraryToolbar

Images

Sync On Sync Off
Screenshot_20240912-232212 Screenshot_20240912-232135

- Add isSyncEnabled parameter to LibraryToolbar and LibraryRegularToolbar
- Pass isSyncEnabled from LibraryTab to LibraryToolbar
- Conditionally display sync library action based on isSyncEnabled
- Update LibraryContent to include isSyncEnabled parameter
- Adjust LibraryTab to handle sync-related functionality
- Remove direct dependency on SyncPreferences in LibraryToolbar
@FuSan21
Copy link
Author

FuSan21 commented Sep 12, 2024

Newbie here, so please check the PR with extra caution.

Currently, it works, but an app restart is needed for changes to show in the library. I'm not sure how to make it work in real-time, so I could use some help with that.

@jobobby04
Copy link
Owner

Looks good otherwise

FuSan21 and others added 3 commits September 12, 2024 23:45
- Replace static isSyncEnabled update with dynamic observation
- Use syncPreferences.syncService().changes() to update isSyncEnabled state
- Ensure isSyncEnabled is updated whenever the sync service changes

Co-authored-by: jobobby04 <[email protected]>
- Resolve ambiguity in lambda parameters
- Correctly update isSyncEnabled state based on syncService value
- Add distinctUntilChanged() to filter out consecutive duplicate emissions
- Simplify mutableState.update lambda for better readability
- Remove redundant boolean comparison in isSyncEnabled assignment
@FuSan21 FuSan21 marked this pull request as ready for review September 12, 2024 18:32
@FuSan21
Copy link
Author

FuSan21 commented Sep 12, 2024

Tested it, and seems to be working as expected

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.

2 participants