Skip to content
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

Merged
merged 6 commits into from
Oct 31, 2023

Conversation

mickael-menu
Copy link
Member

@mickael-menu mickael-menu commented Oct 27, 2023

Added

Navigator

  • The EPUB navigator automatically moves to the next resource when VoiceOver reaches the end of the current one.

Fixed

Navigator

  • Fixed activating the scroll mode when VoiceOver is enabled in the EPUB navigator.

public let typeScale: Double?
public let verticalText: Bool
public let wordSpacing: Double?
public var backgroundColor: Color?
Copy link
Contributor

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?

Copy link
Member Author

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:

/// 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.

Copy link
Contributor

@ettore ettore Oct 27, 2023

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.

Copy link
Member Author

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.

Copy link
Contributor

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).

Copy link
Member Author

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.

@mickael-menu mickael-menu marked this pull request as ready for review October 28, 2023 09:59
@mickael-menu mickael-menu changed the title Fix VoiceOver in the EPUBNavigatorViewController Improve VoiceOver support for the EPUBNavigatorViewController Oct 28, 2023
@mickael-menu mickael-menu requested a review from ettore October 28, 2023 10:01
@mickael-menu mickael-menu temporarily deployed to LCP October 28, 2023 10:01 — with GitHub Actions Inactive
@mickael-menu mickael-menu temporarily deployed to LCP October 28, 2023 10:01 — with GitHub Actions Inactive
@mickael-menu mickael-menu temporarily deployed to LCP October 28, 2023 10:01 — with GitHub Actions Inactive
@mickael-menu mickael-menu temporarily deployed to LCP October 31, 2023 09:10 — with GitHub Actions Inactive
@mickael-menu mickael-menu temporarily deployed to LCP October 31, 2023 09:10 — with GitHub Actions Inactive
@mickael-menu mickael-menu temporarily deployed to LCP October 31, 2023 09:10 — with GitHub Actions Inactive
@mickael-menu mickael-menu merged commit 653a7c0 into develop Oct 31, 2023
6 checks passed
@mickael-menu mickael-menu deleted the fix-voice-over branch October 31, 2023 09:40
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