Add the ability for NVDA to read charts in Excel

[nvaccessAuto]

Reported by elliott94 on 2011-12-08 14:26
When working with data in worksheets, it sometimes becomes necessary to create charts out of several cells in a worksheet.

Unfortunately, NVDA doesn't currently support these. The attached example shows a Bar Chart, with the labeled cells A1, B1, and C1. It would be great to add support for these, if at all possible.
Blocked by #4838

[nvaccessAuto]

Attachment Test_Chart_1.xls added by elliott94 on 2011-12-08 14:27
Description:
A test Bar Chart.

[nvaccessAuto]

Comment 1 by deependra230 on 2014-04-05 19:16
Hello,
I am working on this ticket. I understand the basic functioning of 'excel.py' but i couldn't find anything related to charts' interface on the web. Moreover, there is no default way to navigate inside the chart/graph. Can someone please suggest where to start and how to proceed ?
I know its quite a vague question but i am stuck at the start.

[nvaccessAuto]

Comment 2 by dineshkaushal on 2014-12-01 06:04
Please review the branch in_t1987_f7

https://manish_agrawal@bitbucket.org/manish_agrawal/nvda.git

The problem of excel chart elements being announced multiple times after every alt tab is solved. Thanks to Jamie, the problem is solved using weakref.

[nvaccessAuto]

Comment 5 by mdcurran on 2014-12-04 05:10
Great work again so far.

Some things to change:

1. When object navigating next or previous from a chartElement, you end up on another window outside the chart. At a minimum you should set previous and next properties to None on ExcelchartElementBase. If possible though, it would be cool to implement next and previous properties that actually fetched the next and previous chart element. However this may be impossible to do.
2. I don't think there is a need to provide all those redirect scripts on ExcelChartElementBase. Instead, on the scripts of the chart, set canPropagate to True. As the chart is the parent, NVDA will automatically allow matching on these. For example:

script_reportTitle.canPropagate=True

1. Provide translator comments for all your translation strings.
2. the _select method on ExcelChartElementBase and subclasses should be renamed to something like _getLabel. _select really suggests that it will do something, rather than get something.
3. We have a role_chart in controlTypes. This can be used for the chart's role.

[nvaccessAuto]

Comment 6 by mdcurran on 2014-12-04 06:47
Also, this branch is still based on the work for the NVDA+f7 list in Excel. We should have a discussion soon around how that code can be rewritten to make use of the browse mode abstraction work in #2975, so that Excel can have quick nav to comments and formulas and such, and how that elements list can also list charts. But for now, only the points in my last comment are important.

[nvaccessAuto]

Comment 7 by dineshkaushal on 2014-12-06 04:08
Thank you Mike.

I have made the changes as per your recommendation.

1. previous and next properties for ExcelChartElement are now set to None. Since these elements get focus from excel events, I don't know how to set actual previous and next element.
2. removed the redirect scripts and added canPropogate for those scripts.
3. added translators comments
4. _select renamed to _getChartElementText
5. changed role to ROLE_CHART for ExcelChart
6. added classes for the objects that do some processing. ExcelChartElementBase handles default prompts for other elements.

I need your help for reporting color for the series. I have provided the similar implementation that is found in excel for color reporting, but NVDA announces rgb(red=x,green = y , blue = z).
Please check def script_reportCurrentChartElementColor(self,gesture) in ExcelChartElementBase

the branch is at in_t1987_f7
​https://manish_agrawal@bitbucket.org/manish_agrawal/nvda.git

[nvaccessAuto]

Comment 8 by dineshkaushal on 2014-12-06 04:13
Forgot to ask:

where can I find the code for element list for browse mode? I tried, but I didn't find it in excel.py in branch next and t2975.

[nvaccessAuto]

Comment 9 by mdcurran (in reply to comment 7) on 2014-12-06 07:09
Replying to dineshkaushal:
Great.
For colors, you need to use the RGB object's name property.

ui.message ( _( "Series color: {} ").format(colors.RGB.fromCOLORREF(int( self.excelChartObject.SeriesCollection( self.arg1 ).Interior.Color ) ).name  ) )

[nvaccessAuto]

Comment 10 by mdcurran (in reply to comment 8) on 2014-12-06 07:13
Replying to dineshkaushal:
New abstracted browse mode and elements list is part of #2975. Branch t2975.
However, its only been added to Microsoft Word so far. No work has been done on Excel.
Most of the abstraction is in browseMode.py, but you can see a real implementation in NVDAObjects/window/winword.py.

[nvaccessAuto]

Comment 11 by dineshkaushal (in reply to comment 9) on 2014-12-06 17:43
Fixed the reporting of colors using the name property.
Thank you very much

[nvaccessAuto]

Comment 12 by dineshkaushal (in reply to comment 10) on 2014-12-15 10:13
I have started working on integrating browseMode. I am seeking a clarification.

Do we need to implement TextInfo? as per the following code, it looks that we need to implement TextInfo.

class TextInfoQuickNavItem(QuickNavItem):

def activate(self):
    self.textInfo.activate()

Should we discuss these issues on this ticket or on #2975?
Replying to mdcurran:

Replying to dineshkaushal:

New abstracted browse mode and elements list is part of #2975. Branch t2975.

However, its only been added to Microsoft Word so far. No work has been done on Excel.

Most of the abstraction is in browseMode.py, but you can see a real implementation in NVDAObjects/window/winword.py.

[nvaccessAuto]

Comment 13 by jteh on 2014-12-15 10:39
No, you shouldn't implement a TextInfo. TextInfoQuickNavItem provides a base implementation of several QuickNavItem methods to avoid duplication if you do have a TextInfo. If you don't, you inherit from QuickNavItem directly and implement those methods (such as ```lt_}} (less than comparison), label, activate, isChild, moveTo, etc.) yourself. See the QuickNavItem class for details.

[nvaccessAuto]

Comment 14 by dineshkaushal (in reply to comment 13) on 2014-12-15 12:56
Then will I also implement

class BrowseModeTreeInterceptor(treeInterceptorHandler.TreeInterceptor):

Because script BrowseModeTreeInterceptor.activatePosition calls self._activatePosition(info)

[nvaccessAuto]

Comment 15 by jteh on 2014-12-15 22:33
Hmm. We'll need to discuss that. For now, just subclass BrowseModeTreeInterceptor. If you want enter to work, just override script_activatePosition. If we change anything related to this, it won't have a significant impact on your code.

[nvaccessAuto]

Comment 16 by dineshkaushal (in reply to comment 15) on 2014-12-22 09:14

I have done first implementation, but I am unable to make the code work. After implementing the code, no excel keys are working.
branch in_t1987_bm
​https://manish_agrawal@bitbucket.org/manish_agrawal/nvda.git

[nvaccessAuto]

Attachment nvda.log added by dineshkaushal on 2015-01-05 14:01
Description:
For comment 17

[nvaccessAuto]

Comment 17 by dineshkaushal on 2015-01-05 14:03
The current implementation works somewhat, but there is an error on selecting current chart. The following error is there.

AttributeError: 'ExcelBrowseModeTreeInterceptor' object has no attribute 'makeTextInfo'

See the attached log file for more details.

branch in_t1987_bm
​​https://manish_agrawal@bitbucket.org/manish_agrawal/nvda.git

[nvaccessAuto]

Comment 18 by mdcurran on 2015-01-09 02:43
Just a heads up that QuicknavItem has been slitely changed in master, e7c90a3 to fix #4767. This will affect this ticket, so you'll want to merge master and made the necessary change.

• QuickNavItem.moveTo now only should do the move, but not any reporting. It also therefore no longer takes any keyword arguments.
• QuicknavItem.report method has been introduced, this method should handle reporting of the item. It takes a readUnit keyword argument like moveTo did, which you may or may not use.

[nvaccessAuto]

Comment 19 by dineshkaushal on 2015-01-21 05:55
I merged master with in_t1987_bm, but I could not find the change.

I then checked out master, and I can still not find the changed definition for QuickNavItem in browseMode.py file.

Which branch should I checkout?

[nvaccessAuto]

Comment 20 by mdcurran (in reply to comment 19) on 2015-01-21 06:16
Replying to dineshkaushal:
A current checkout of master for me, source/browsemode.py: the new report method starts at line 103.
The moveTo method is at line 111.

[nvaccessAuto]

Comment 21 by dineshkaushal on 2015-01-21 10:10
I got the changes from master, but I am still getting the error for MakeTextInfo

[nvaccessAuto]

Comment 22 by mdcurran (in reply to comment 21) on 2015-01-21 22:47
Replying to dineshkaushal:
The makeTextInfo error is not related to the other changes. However, I'll look at that makeTextInfo bug today. I meant to do this a while ago.

[nvaccessAuto]

Comment 24 by mdcurran (in reply to comment 22) on 2015-01-22 01:57
Replying to mdcurran:
Please merge t4838 into your branch. This will stop the makeTextInfo error.
Also a few other things:

• In my copy of in_t1987_bm, it seems excelChart is not imported in source/NVDAObjects/window/excel.py. You won't be able to import excelChart at the top as you'll get a circular import error as excelChart imports excel and tries to use excelBase. But instead you can import excelChart in _getSelection just before you use excelChart.ExcelChart.

[nvaccessAuto]

Comment 25 by dineshkaushal on 2015-01-29 09:13
Added chart, comment and formula listing options in the browse mode.

branch in_t1987_bm
https://manish_agrawal@bitbucket.org/manish_agrawal/nvda.git

[nvaccessAuto]

Comment 26 by mdcurran on 2015-02-04 23:27
Some final code review:

• As you've initially copied some of the QuicknavItem stuff from winword.py, some of the docstrings and comments are still mentioning Microsoft Word. Please rewrite these to refer to Excel.
• It may be an idea to rename collectionFromRange to collectionFromWorksheet as that is what it now deals with in Excel.
• Would it be an idea to create a base class for ExcelCommentQuickNavItem, ExcelFormulaQuickNavItem etc, that implements __lt__ moveTo, and isAfterSelection, so that the same code isn't duplicated for all those classes? Though, please excuse me if the code in each is slightly different. If so then don't worry about this. But from a quick glance it looked pretty much the same.

[nvaccessAuto]

Comment 27 by dineshkaushal on 2015-02-09 10:16
Thanks Mike,
I have fixed all the doc strings refering to word, created ExcelRangeBasedQuickNavItem class as base class for ExcelCommentQuickNavItem and ExcelFormulaQuickNavItem, and renamed collectionFromRange to collectionFromWorksheet.

[nvaccessAuto]

Comment 28 by mdcurran on 2015-02-10 02:17
I've now branched in_t1987_bm to t1987 on NV Access.
I've made the following changes:

• Fixed broken submodules. When pulling or merging from this branch, do a git submodule update after to ensure you have the correct submodules and that you don't accidentally commit old ones.
• Excel's treeInterceptor now is created always, though is set to passThrough by default. Also the elementsList script has been overridden on the treeInterceptor to allow it to work in passthrough mode. In short, now when in Excel, you can press NVDZA+f7 to get the elemts list with out having to press NVDA+space first.
• I rewrote ExcelQuickNavIterator.iterate as the code was picking up some incorrect cells due to Excel's rante object's not handling item indexing in a good way. It seems indexing simply accesses adjacent cells in the sheet, rather than in the actual range. Therefore, for now we just iterate over the range itself. This works fine for the elements list, but this would have to be rethought if we wanted to start supporting quick navigation keys from arbitrary locations in a sheet.
• Excel's treeInterceptor now only stays alive while this worksheet is the active sheet. This means that when switching sheets, elements list will show the correct data for the sheet.
  • NVDA now fires a focus event in moveTo for comments/formulars/charts, as Excel's activate method does not do this itself. Therefore, now NVDA announces hat you land on after moving to it from the elements list.

Take a look at my work and provide any feedback. If everyone is happy, then I can prepare to merge this into next.

[nvaccessAuto]

Comment 29 by dineshkaushal on 2015-02-12 15:45
Testing is done, you can now integrate this with next.

Thanks for your help.

[nvaccessAuto]

Comment 30 by Michael Curran <mick@... on 2015-02-13 10:14
In [f372f6e]:

Merge branch 't1987' into next. Incubates #1987

Changes:
Added labels: incubating

[nvaccessAuto]

Comment 31 by Michael Curran <mick@... on 2015-02-14 13:19
In [99683fb]:

Merge branch 't1987' into next. Incubates #1987

[nvaccessAuto]

Comment 33 by mdcurran on 2015-03-16 05:03
were there any outstanding bugs on this implementation? I thought one was mentioned on a call but can't quite remember.

[nvaccessAuto]

Comment 34 by dineshkaushal on 2015-04-07 10:52
Modified the code, changed shortcut for comments from c to o.

get the code from in_t1987_temp

[nvaccessAuto]

Comment 35 by Michael Curran <mick@... on 2015-04-14 00:24
In [dc92f31]:

Merge branch 't1987' into next. Incubates #1987

[nvaccessAuto]

Comment 36 by JamaicanUser on 2015-04-23 03:20
Will this functionality be available for the next release? I find it very handy.

[nvaccessAuto]

Comment 37 by jteh on 2015-04-23 03:26
Barring any major issues, yes. See ReleaseProcess.

[nvaccessAuto]

Comment 38 by Michael Curran <mick@... on 2015-04-29 06:18
In [410b90b]:

Merge branch 't1987'. Fixes #1987

Changes:
Removed labels: incubating
State: closed

[nvaccessAuto]

Comment 39 by JamaicanUser on 2015-05-01 11:50
Don't know where to report this, but when I enter Input Help mode in Excel and press the Elements List dialog command (which I think should be renamed in the User guide to Microsoft Excel Elements List to avoid confusion), the description states headings, links and landmarks. This should say charts, cells with comments and cells with formulas.

[nvaccessAuto]

Comment 40 by James Teh <jamie@... on 2015-05-01 12:12
In [3625417]:

Fix the command description for the Elements List command in Microsoft Excel, which previously incorrectly mentioned links, headings and landmarks.

Re #1987.

[nvaccessAuto]

Comment 41 by JamaicanUser on 2015-05-02 15:31
Is there a special reason this is not listed under the 2015.2 milestone fixes?

[nvaccessAuto]

Comment 42 by ondrosik on 2015-05-06 13:00
does this work for excel 2003? When I look at the example chart, i see one chart but for some reason it says it is placed on e13 m 38. When I press enter, It says the name, type, and testcel1, testcel 2 but I can not navigate. I tryed also another random chart from the internet with same result e. g. the navigation by arrow keys doesn't work. but maybe I am missing something important, I newer used charts in ms excel at all. I am testing this for translation purposes.

[nvaccessAuto]

Comment 43 by ondrosik on 2015-05-07 17:22
OK, I werified it, it works with office 2007. So the question is: Will we support 2003 or will we state that this is supported in 2007 and newer? I know that office 2003 is really old piece of software, I use it because I hate rybbons so it is mostly my lazyness.

[nvaccessAuto]

Attachment a.xlsx added by peter on 2015-05-13 08:20
Description:

[nvaccessAuto]

Comment 44 by peter on 2015-05-13 08:23
Hello,
I have one particular document where i canot see list of formulas. Steps to reproduce:

1. open attachment attached above
2. press nvda+f7 and choose formula
3. nvda will now freeze for few seconds so wait a moment and then check the list of formulas, it's empty.
   I have latest master branch and office 2010.

[nvaccessAuto]

Comment 45 by ondrosik on 2015-05-14 08:12
I can reproduce this with Excel 2007 but it seems that sometimes I see the formulas in the list, sometimes not. It seems that it is somehow related with the cursor position. When i move the cursor around in some cases i get the list with formulas. I thought that it depends on particular column but it doesn't when i am on A4 i sometimes get the formulas, sometimes don't.
I am providing log for both cases but be avare that it is too long, see attached file.

[nvaccessAuto]

Attachment attachment-comment-45.7z added by ondrosik on 2015-05-14 08:16
Description:
Comment 45