Skip to content

change order of informations presented by CTRL+NVDA+f1, to make changeable info presented first.#8927

Merged
feerrenrut merged 9 commits into
nvaccess:masterfrom
lukaszgo1:I7338
May 10, 2019
Merged

change order of informations presented by CTRL+NVDA+f1, to make changeable info presented first.#8927
feerrenrut merged 9 commits into
nvaccess:masterfrom
lukaszgo1:I7338

Conversation

@lukaszgo1

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #7338

Summary of the issue:

When pressing CTRL+NVDA+F1 the order of information's is not optimal.

Description of how this pull request fixes the issue:

The changeable parts i.e. application name, and app module name are now presented first.

Testing performed:

Tested, that script still present the same info, but in the different order.

Known issues with pull request:

None

Change log entry:

This command isn't even included in the user guide, so i believe, that Change log entry isn't nesesary.

@jscholes

jscholes commented Nov 7, 2018

Copy link
Copy Markdown

I think this change itself is minor, and as you mentioned the command isn't mentioned in the user guide. However, there is no seperator between your two messages, be it a comma, a similar character or the word "and". This results in strings like:

explorer.exe is currently running Explorer module is loaded

with the information running together.

@lukaszgo1

Copy link
Copy Markdown
Contributor Author

@jscholes Thanks for your commends. I assumed, that the space at the beginning of the second message would be sufficient, but adding a dot after the first looks like a good solution.

@LeonarddeR

Copy link
Copy Markdown
Collaborator

Now while we're at it, I think I prefer swapping the order as well. First report the app module, than the executable name. This makes it more intuitive to hear whether a specific appmodule is loaded for an app.

@lukaszgo1

Copy link
Copy Markdown
Contributor Author

It'd be nice to have this reviewed.

@feerrenrut

Copy link
Copy Markdown
Contributor

It'd be nice to have this reviewed.

I'm looking at it now. In the future, please use the "request review" button. Though saying that, do you have access to it?

@lukaszgo1

Copy link
Copy Markdown
Contributor Author

No i don't as per this commend in the GitHub documentation

Note: Pull request authors can't request reviews unless they are either a repository owner or collaborator with write access to the repository.

@feerrenrut

Copy link
Copy Markdown
Contributor

Ah, I see. My apologies.

Comment thread source/globalCommands.py Outdated
@LeonarddeR

This comment has been minimized.

@lukaszgo1

Copy link
Copy Markdown
Contributor Author

@feerrenrut I believe I have addressed your commend.

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @lukaszgo1

@feerrenrut

Copy link
Copy Markdown
Contributor

There should still be a change log entry. I will add something along the lines of:

changes for developers

When using the report module info command, the order of information has changed to present the module first. (#7338)

@feerrenrut feerrenrut merged commit 4b96ee3 into nvaccess:master May 10, 2019
@nvaccessAuto nvaccessAuto added this to the 2019.2 milestone May 10, 2019
feerrenrut added a commit that referenced this pull request May 10, 2019
@lukaszgo1 lukaszgo1 deleted the I7338 branch May 13, 2019 15:55
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.

ctrl+NVDA+f1 Speech Order

5 participants