-
Notifications
You must be signed in to change notification settings - Fork 773
Map scroll directions to actual segment indices in fonts menu #238
Conversation
@@ -243,7 +243,12 @@ class FolioReaderFontsMenu: UIViewController, SMSegmentViewDelegate, UIGestureRe | |||
scrollDirection = readerConfig.scrollDirection | |||
} | |||
|
|||
layoutDirection.selectSegmentAtIndex(scrollDirection?.rawValue ?? 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure that this is the cause of the crash? Because it fallback to 0 if is nil.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not nil
. It's 3
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried on PR #237 because I am currently reviewing that and this is not happening, how do I reproduce that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It won't crash if you have different defaults already set for configuration. See this line:
It sets the default value to defaultVertical
. In this line it takes the default value if nothing has changed it:
And then the conditional in this line does not get triggered:
Which means that when you reach this line:
scrollDirection
is still defaultVertical
, which has a rawValue
of 3. There's only two segments so it crashes.
I expect you have some unique state from developing the project that set your current scroll direction to either horizontal
or vertical
. I think user defaults are maintained across app deletion, so to reproduce you would have to either
- manually clear out user defaults programmatically, then relaunch to let the defaults get set again, or
- reset a simulator to start fresh on it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, you are right :)
@@ -243,7 +243,12 @@ class FolioReaderFontsMenu: UIViewController, SMSegmentViewDelegate, UIGestureRe | |||
scrollDirection = readerConfig.scrollDirection | |||
} | |||
|
|||
layoutDirection.selectSegmentAtIndex(scrollDirection?.rawValue ?? 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, you are right :)
This is intended to fix the crash in #215. The only options in the menu are vertical and horizontal, so the UI excludes the horizontalWithVerticalContent option, but that option seems to be only configurable programmatically and is incompatible with letting the user select a scroll direction.