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

Map scroll directions to actual segment indices in fonts menu #238

Merged
merged 1 commit into from
May 29, 2017
Merged

Map scroll directions to actual segment indices in fonts menu #238

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion Source/FolioReaderFontsMenu.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,12 @@ class FolioReaderFontsMenu: UIViewController, SMSegmentViewDelegate, UIGestureRe
scrollDirection = readerConfig.scrollDirection
}

layoutDirection.selectSegmentAtIndex(scrollDirection?.rawValue ?? 0)
Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Member

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?

Copy link
Contributor Author

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:

https://github.com/FolioReader/FolioReaderKit/pull/237/files#diff-7d83fdc08c25740f0dc3bab59bd7c2bbL72

It sets the default value to defaultVertical. In this line it takes the default value if nothing has changed it:

https://github.com/FolioReader/FolioReaderKit/pull/237/files#diff-21569209c801652e20649d5072f824d0R255

And then the conditional in this line does not get triggered:

https://github.com/FolioReader/FolioReaderKit/pull/237/files#diff-21569209c801652e20649d5072f824d0R255

Which means that when you reach this line:

https://github.com/FolioReader/FolioReaderKit/pull/237/files#diff-21569209c801652e20649d5072f824d0R255

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

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, you are right :)

switch scrollDirection ?? .vertical {
case .vertical, .defaultVertical:
layoutDirection.selectSegmentAtIndex(FolioReaderScrollDirection.vertical.rawValue)
case .horizontal, .horizontalWithVerticalContent:
layoutDirection.selectSegmentAtIndex(FolioReaderScrollDirection.horizontal.rawValue)
}
menuView.addSubview(layoutDirection)
}

Expand Down