Skip to content

Conversation

mickael-menu
Copy link
Member

@mickael-menu mickael-menu commented Aug 9, 2022

Review notes

The Settings API PRs are numbered in the order of merge, but should be reviewed in reverse order, starting with #141.

However, you might want to start by reading the documentation that is in this PR, as it is the most up-to-date:

This PR:

  • replaces the legacy user settings in the EPUB navigator with the new ones in a backward compatible way
  • adds the migration guide

@mickael-menu mickael-menu force-pushed the feature/settings+replace-legacy branch from 9660919 to c2ccd13 Compare August 11, 2022 13:21
@mickael-menu mickael-menu force-pushed the feature/settings+replace-legacy branch from f906a5f to 680d74a Compare August 12, 2022 14:36
@mickael-menu mickael-menu force-pushed the feature/settings+replace-legacy branch from 680d74a to fb34c59 Compare August 12, 2022 15:14
@mickael-menu mickael-menu force-pushed the feature/settings+replace-legacy branch from 5509e6a to 1eee3d8 Compare August 12, 2022 16:12
@mickael-menu mickael-menu force-pushed the feature/settings+replace-legacy branch from 9bef708 to 9748c7e Compare August 16, 2022 13:07

resourcePager.adapter = adapter
resourcePager.direction = publication.metadata.effectiveReadingProgression
resetAdapter()
Copy link
Member Author

Choose a reason for hiding this comment

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

The adapter can now be reset when changing settings that require a relayout (e.g. reading progression).

private lateinit var resourcesSingle: List<PageResource>
private lateinit var resourcesDouble: List<PageResource>

lateinit var preferences: SharedPreferences
Copy link
Member Author

Choose a reason for hiding this comment

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

The EpubNavigatorFragment is in "legacy mode" (with SharedPreferences settings) if a baseUrl is provided. If it is null, we switch to the new Settings API.


override val readingProgression: ReadingProgression
get() = publication.metadata.effectiveReadingProgression
get() = viewModel.readingProgression
Copy link
Member Author

Choose a reason for hiding this comment

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

The readingProgression is now dynamic as it's also a setting.

@mickael-menu mickael-menu requested a review from qnga August 16, 2022 15:34
@mickael-menu mickael-menu marked this pull request as ready for review August 16, 2022 15:34
@mickael-menu mickael-menu requested a review from qnga September 7, 2022 11:05
Copy link
Member

@qnga qnga left a comment

Choose a reason for hiding this comment

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

It's hard to say that I saw and thought about everything, but let's go. I might have more insight after working on PDF settings.

@mickael-menu mickael-menu merged commit cb69b6d into feature/settings+server Sep 7, 2022
@mickael-menu mickael-menu deleted the feature/settings+replace-legacy branch September 7, 2022 13:31
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