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

Add a script to browse mode to toggle inclusion of layout tables #7634

Merged
merged 2 commits into from Oct 31, 2017

Conversation

@leonardder
Copy link
Collaborator

@leonardder leonardder commented Sep 29, 2017

Link to issue number:

None

Summary of the issue:

Some tables on the web, even though they have data, are treated as layout tables as they don't follow the convention that headers are required for a table to be treated as a data table. It is currently not possible to switch inclusion of layout tables on the fly while in browse mode.

Description of how this pull request fixes the issue:

Adds an unbound script to browse mode to toggle layout tables. Even though this is a document formatting setting according to the config spec, I made this browse mode only due to the fact that from a UX perspectieve, this is a browse mode setting.

Testing performed:

Toggled this setting while in a layout table in Firefox browse mode, detection changed on the fly as expected.

Known issues with pull request:

None i'm aware of

Change log entry:

  • Changes
    • An unbound command has been added for browse mode to toggle the inclusion of layout tables on the fly. You can find this command in the Browse mode category of the Input Gestures dialog.
@derekriemer
Copy link
Collaborator

@derekriemer derekriemer commented Sep 30, 2017

Nice work!

Copy link
Collaborator

@derekriemer derekriemer left a comment

Nice work!

def script_toggleIncludeLayoutTables(self,gesture):
if config.conf["documentFormatting"]["includeLayoutTables"]:
# Translators: The message announced when toggling the include layout tables document formatting setting.
state = _("include layout tables off")

This comment has been minimized.

@derekriemer

derekriemer Sep 30, 2017
Collaborator

Maybe "layout tables off" or "layout tables disabled" might be more easy to understand.

This comment has been minimized.

@leonardder

leonardder Sep 30, 2017
Author Collaborator

I went the layout tables off route. Also fixed the translator comments in the process.

config.conf["documentFormatting"]["includeLayoutTables"]=True
ui.message(state)
# Translators: Input help mode message for include layout tables command.
script_toggleIncludeLayoutTables.__doc__=_("Toggles on and off the inclusion of layout tables in browse mode")

This comment has been minimized.

@derekriemer

derekriemer Sep 30, 2017
Collaborator

Do you need a category attribute here? I'm not familiar with this code.

This comment has been minimized.

@leonardder

leonardder Sep 30, 2017
Author Collaborator

Nope, the category is set on the class itself, so all the scripts have the category that the class prescribes.

michaelDCurran added a commit that referenced this pull request Oct 17, 2017
@feerrenrut feerrenrut removed their request for review Oct 18, 2017
@michaelDCurran michaelDCurran merged commit 36beb96 into nvaccess:master Oct 31, 2017
@nvaccessAuto nvaccessAuto added this to the 2017.4 milestone Oct 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants