-
Notifications
You must be signed in to change notification settings - Fork 126
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
Improve VoiceOver support for the EPUBNavigatorViewController
#349
Conversation
public let typeScale: Double? | ||
public let verticalText: Bool | ||
public let wordSpacing: Double? | ||
public var backgroundColor: Color? |
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.
would it be possible to add some code documentation to explain the difference (conceptually) between EPUBSettings
, EPUBPreferences
, EPUBDefaults
? they seem closely related. Or maybe it already exists and I am missing 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.
Could you take a look at this and tell me if that helps to understand it?
https://github.com/readium/architecture/blob/master/proposals/009-preferences-api.md#overview
There's also this, but maybe it's not clear enough:
swift-toolkit/Sources/Navigator/Preferences/Configurable.swift
Lines 19 to 24 in 4a9a4d9
/// Submits a new set of `Preferences` to update the current `Settings`. | |
/// | |
/// Note that the `Configurable` might not update its `settings` right away, or might even | |
/// ignore some of the provided preferences. They are only used as hints to compute the new | |
/// settings. | |
func submitPreferences(_ preferences: Preferences) |
Generally, you can ignore XSettings
as an integrator. It will be used by the EPUBPreferencesEditor
.
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.
Could you take a look at this and tell me if that helps to understand it? https://github.com/readium/architecture/blob/master/proposals/009-preferences-api.md#overview
This is great! I didn't know it existed. IMO it would be great to have a link to this in the code where the related types (EPUBPreferences, EPUBSettings, EPUBDefaults) are defined. I can also contribute that if not clear.
There's also this, but maybe it's not clear enough:
This is useful too (although a bit buried) but not as foundational as the documentation you linked.
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.
Actually we have even better, there's a Swift-specific user guide for that here: https://github.com/readium/swift-toolkit/blob/main/Documentation/Guides/Navigator%20Preferences.md#configuring-the-navigator
That could be useful to link to this markdown. Ideally we could add something like this <doc:Guides/NavigatorPreferences>
but I think we need to setup a DocC documentation catalog for this first.
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.
I was even thinking we could just add that URL to the class doc, i.e. at line 12 of EPUBSettings
(or equivalent in EPUBPreferences
).
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.
Sure that could be enough until we add a real DocC catalog.
EPUBNavigatorViewController
EPUBNavigatorViewController
74fe4d8
to
585677a
Compare
Added
Navigator
Fixed
Navigator