Skip to content
This repository has been archived by the owner on Nov 26, 2020. It is now read-only.

Fix scroll direction being ignored on first run #178

Merged

Conversation

revolter
Copy link
Member

Setting currentScrollDirection to .horizontal was ignored before the
user changed it manually. Now, setting it like mentioned, the book
scrolls horizontally as expected, and if the user changes it to
vertical, it is respected by the app. Not setting the property still
scrolls vertically by default, as before.

Setting currentScrollDirection to .horizontal was ignored before the
user changed it manually. Now, setting it like mentioned, the book
scrolls horizontally as expected, and if the user changes it to
vertical, it is respected by the app. Not setting the property still
scrolls vertically by default, as before.
@hebertialmeida hebertialmeida self-assigned this Nov 15, 2016
@hebertialmeida
Copy link
Member

I am not sure about this, how can I reproduce this behavior?

@revolter
Copy link
Member Author

revolter commented Dec 9, 2016

This is easily reproducible by creating a sample app and setting the FolioReaderConfig's scrollDirection to . horizontal . On the very first run of the app, the scrolling will be vertically instead. After the user changes it to horizontal, it is correctly persisted and loaded.

More easy than this is to open your example app, and, IIRC, none of the 2 books are being able to be scrolled horizontally on the first run, even though it's set this way for one of them.

@@ -69,7 +69,7 @@ open class FolioReaderContainer: UIViewController {
kCurrentHighlightStyle: 0,
kCurrentTOCMenu: 0,
kCurrentMediaOverlayStyle: MediaOverlayStyle.default.rawValue,
kCurrentScrollDirection: FolioReaderScrollDirection.vertical.rawValue
kCurrentScrollDirection: FolioReaderScrollDirection.defaultVertical.rawValue
Copy link
Member Author

Choose a reason for hiding this comment

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

This makes sure the initial value of the scroll direction is the new .defaultVertical, and not .vertical. This default value works as vertical if the user doesn't sets another value for the config's scroll direction.

@@ -95,8 +95,13 @@ open class FolioReaderContainer: UIViewController {

// If user can change scroll direction use the last saved
if readerConfig.canChangeScrollDirection {
let direction = FolioReaderScrollDirection(rawValue: FolioReader.currentScrollDirection) ?? .vertical
readerConfig.scrollDirection = direction
var scrollDirection = FolioReaderScrollDirection(rawValue: FolioReader.currentScrollDirection) ?? .vertical
Copy link
Member Author

Choose a reason for hiding this comment

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

This loads the persisted scroll direction, which, in the first app run, is .defaultVertical as set by the line #72.

readerConfig.scrollDirection = direction
var scrollDirection = FolioReaderScrollDirection(rawValue: FolioReader.currentScrollDirection) ?? .vertical

if (scrollDirection == .defaultVertical && readerConfig.scrollDirection != .defaultVertical) {
Copy link
Member Author

@revolter revolter Dec 9, 2016

Choose a reason for hiding this comment

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

If the persisted value is .defaultVertical, meaning this is the first app run, and the user has set a scroll direction different from the default one, use the user's scroll direction.

@@ -234,7 +234,14 @@ class FolioReaderFontsMenu: UIViewController, SMSegmentViewDelegate, UIGestureRe
layoutDirection.tag = 3
layoutDirection.addSegmentWithTitle(readerConfig.localizedLayoutVertical, onSelectionImage: verticalSelected, offSelectionImage: verticalNormal)
layoutDirection.addSegmentWithTitle(readerConfig.localizedLayoutHorizontal, onSelectionImage: horizontalSelected, offSelectionImage: horizontalNormal)
layoutDirection.selectSegmentAtIndex(Int(FolioReader.currentScrollDirection))

var scrollDirection = FolioReaderScrollDirection(rawValue: FolioReader.currentScrollDirection)
Copy link
Member Author

Choose a reason for hiding this comment

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

Can't remember why didn't it require me to add ?? .vertical here too, as on the line #98.


var scrollDirection = FolioReaderScrollDirection(rawValue: FolioReader.currentScrollDirection)

if scrollDirection == .defaultVertical && readerConfig.scrollDirection != .defaultVertical {
Copy link
Member Author

Choose a reason for hiding this comment

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

Same as this comment.

@hebertialmeida hebertialmeida merged commit 5b76502 into FolioReader:master Dec 9, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants