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

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

Nice work!

Copy link
Collaborator

@derekriemer derekriemer left a comment

Choose a reason for hiding this comment

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

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")
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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")
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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 October 18, 2017 09:45
@michaelDCurran michaelDCurran merged commit 36beb96 into nvaccess:master Oct 31, 2017
@nvaccessAuto nvaccessAuto added this to the 2017.4 milestone Oct 31, 2017
@LeonarddeR LeonarddeR added the BabbageWork Pull requests filed on behalf of Babbage B.V. label Oct 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BabbageWork Pull requests filed on behalf of Babbage B.V. feature/browse-mode quick fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants