Merge pull request #3 from make4all/vpotluri/pre-review-cleanup

Addressed feedback from NVDA

Author: LuckyQXW

addon/appModules/chrome/__init__.py

@@ -13,6 +13,7 @@
 import review
 import wx
 import gui
+import textInfos
 import threading
 from .spritesTable import SpritesTable
 from .dialogs import SearchDialog
@@ -98,13 +99,12 @@ def post_handleCaretMove(self, pos, *args, **kwargs):
 		'''
 		try:
 			original_handleCaretMove(pos, *args, **kwargs)
-			nav = api.getNavigatorObject()
-			tableInfo = self.getTableInfo(nav)
+			tableInfo = self.getTableInfo()
 			if tableInfo and not self.hasAnnouncedTable and not self.spritesMode:
 				self.hasAnnouncedTable = True
 				# Translators: message announcing sprites mode is available upon entry to a table
 				ui.message(_('Sprites mode available. Press NVDA+Shift+T to activate.'))
-				self.logHelper.logTableFound(None, None, tableInfo[0].rowCount, tableInfo[0].columnCount)
+				self.logHelper.logTableFound(None, None, tableInfo[1], tableInfo[2])
 			if not tableInfo:
 				self.hasAnnouncedTable = False
 		except Exception:
@@ -152,12 +152,11 @@ def script_spritesOn(self, gesture):
 		Toggle sprites mode on and set up the table state and gestures
 		'''
 		try:
-			nav = api.getNavigatorObject()
 			# retreive the table object and tableID if available
-			tableInfo = self.getTableInfo(nav)
+			tableInfo = self.getTableInfo()
 			if tableInfo:
-				tableObj, tableID = tableInfo
-				self.table = SpritesTable(tableObj, tableID, len(self.rowKeys), len(self.columnKeys))
+				tableID, rowCount, columnCount = tableInfo
+				self.table = SpritesTable(tableID, rowCount, columnCount, len(self.rowKeys), len(self.columnKeys))
 				# Translators: message announcing sprites mode activation and the key to exit
 				ui.message(_('Sprites mode on. Press escape to exit. Press F to enter search mode.'))
 				# bind gestures to the table
@@ -167,7 +166,7 @@ def script_spritesOn(self, gesture):
 				self.spritesID = int(config.conf['sprites']['spritesID']) + 1
 				config.conf['sprites']['spritesID'] = self.spritesID
 				self.logHelper.logSpritesToggle(self.spritesID, None, 'on')
-				self.logHelper.logTableInfo(self.spritesID, None, tableObj.rowCount, tableObj.columnCount)
+				self.logHelper.logTableInfo(self.spritesID, None, rowCount, columnCount)
 				self.speakMapping()
 				self.spritesMode = True
 			else:
@@ -177,7 +176,7 @@ def script_spritesOn(self, gesture):
 			self.logHelper.logErrorException(None, None, traceback.format_exc())
 
 
-	def getTableInfo(self, nav):
+	def getTableInfo(self):
 		'''
 		Returns the table object and the tableID associated with the given navigator object
 		Returns None if the navigator is currently not in a table
@@ -186,20 +185,22 @@ def getTableInfo(self, nav):
 		try:
 			selection = focus.treeInterceptor.selection
 			tableID, origRow, origCol, origRowSpan, origColSpan = focus.treeInterceptor._getTableCellCoords(selection)
+			if selection.isCollapsed:
+				selection = selection.copy()
+				selection.expand(textInfos.UNIT_CHARACTER)
+			fields = list(selection.getTextWithFields())
+			for field in fields:
+				if not (isinstance(field, textInfos.FieldCommand) and field.command == "controlStart"):
+					# Not a control field.
+					continue
+				attrs = field.field
+				tableID = attrs.get('table-id')
+				if tableID is not None:
+					break
+			return (attrs["table-id"],
+				int(attrs["table-rowcount"]), int(attrs["table-columncount"]))
 		except (AttributeError, LookupError):
 			return None
-		try:
-			while nav.parent and not nav.table:
-				nav = nav.parent
-		except NotImplementedError:
-			return None
-		if not nav.parent:
-			return None
-		# found table
-		if nav.role == controlTypes.ROLE_TABLE:
-			return nav, tableID
-		elif nav.table:
-			return nav.table, tableID
 
 	def bindSpritesGesture(self):
 		'''
@@ -429,7 +430,7 @@ def speakCell(self, row=None, column=None):
 		currRow, currColumn = self.table.getCurrPosition()
 		if self.table.hasResultAt(currRow, currColumn):
 			ui.message(_('Found'))
-		speech.speakTextInfo(info, formatConfig=formatConfig, reason=controlTypes.REASON_CARET)
+		speech.speakTextInfo(info, formatConfig=formatConfig, reason=controlTypes.OutputReason.CARET)
 		# Announce other occurrences on the same row/column
 		# if the given position contains search result, would exclude current position
 		# when calculating the occurrences

addon/appModules/chrome/spritesTable.py

@@ -11,14 +11,14 @@ class SpritesTable():
 	'''
 	A class keeping track of the states of the sprites table
 	'''
-	def __init__(self, tableObj, tableID, rowKeyCount, columnKeyCount, test=False):
+	def __init__(self, tableID, rowCount, columnCount, rowKeyCount, columnKeyCount, test=False):
 		self.tableID = tableID
 		if not test:
 			self.interceptor = api.getFocusObject().treeInterceptor
 
 		# stores the initial row and column count for the table object
-		self.rowCount = tableObj.rowCount
-		self.columnCount = tableObj.columnCount
+		self.rowCount = rowCount
+		self.columnCount = columnCount
 		# list of row and column index including all rows and columns
 		self.allRows = [r + 1 for r in range(self.rowCount)]
 		self.allColumns = [c + 1 for c in range(self.columnCount)]
@@ -70,7 +70,7 @@ def getCellAt(self, row=None, column=None):
 		info = self.interceptor._getTableCellAt(self.tableID, selection, self.currRow, self.currColumn)
 		# move browse mode selection
 		self.interceptor.selection = info
-		log.info(f'switch to row {self.currRow} column {self.currColumn}')
+		# log.info(f'switch to row {self.currRow} column {self.currColumn}')
 		return info
 
 	def changeRowOffset(self, delta):
@@ -166,8 +166,6 @@ def searchTable(self, keyword, caseSensitive, filterRows, filterColumns, onCompl
 					if m:
 						foundCells.append((r, c))
 					startPos = info
-			log.info('logging found rows and columns')
-			log.info(foundCells)
 			if foundCells:
 				self.searchResults = foundCells
 				self.searchTerm = keyword

addon/appModules/chrome/tests.py

@@ -1,16 +1,9 @@
 import unittest
 from spritesTable import SpritesTable
 
-
-class DummyTable:
-	def __init__(self, rowCount, columnCount):
-		self.rowCount = rowCount
-		self.columnCount = columnCount
-
-
 class TestSpritesTable(unittest.TestCase):
 	def testInit(self):
-		t = SpritesTable(DummyTable(3, 4), 1, 5, 13, True)
+		t = SpritesTable(1, 3, 4, 5, 13, True)
 		self.assertEqual(t.tableID, 1)
 		self.assertEqual(t.rowCount, 3)
 		self.assertEqual(t.columnCount, 4)
@@ -32,7 +25,7 @@ def testInit(self):
 		self.assertFalse(t.filterColumns)
 
 	def testChangeRowOffset(self):
-		t = SpritesTable(DummyTable(3, 4), 1, 5, 13, True)
+		t = SpritesTable(1, 3, 4, 5, 13, True)
 		t.changeRowOffset(2)
 		self.assertEqual(t.currRow, 3)
 		t.changeRowOffset(-1)
@@ -42,7 +35,7 @@ def testChangeRowOffset(self):
 		self.assertEqual(t.currRow, 1)
 
 	def testChangeColumnOffset(self):
-		t = SpritesTable(DummyTable(3, 4), 1, 5, 13, True)
+		t = SpritesTable(1, 3, 4, 5, 13, True)
 		t.changeColumnOffset(3)
 		self.assertEqual(t.currColumn, 4)
 		t.changeColumnOffset(-1)
@@ -52,7 +45,7 @@ def testChangeColumnOffset(self):
 		self.assertEqual(t.currColumn, 1)
 
 	def testScrollRowsRegular(self):
-		t = SpritesTable(DummyTable(6, 4), 1, 5, 13, True)
+		t = SpritesTable(1, 6, 4, 5, 13, True)
 		# cannot scroll to previous at start
 		self.assertFalse(t.scroll(direction=0, keyIdx=0))
 		self.assertTrue(t.scroll(direction=0, keyIdx=4))
@@ -66,7 +59,7 @@ def testScrollRowsRegular(self):
 
 	def testScrollRowsEdge(self):
 		# test table with 10 rows
-		t = SpritesTable(DummyTable(10, 4), 1, 5, 13, True)
+		t = SpritesTable(1, 10, 4, 5, 13, True)
 		# cannot scroll to previous at start
 		self.assertFalse(t.scroll(direction=0, keyIdx=0))
 		self.assertTrue(t.scroll(direction=0, keyIdx=4))
@@ -80,7 +73,7 @@ def testScrollRowsEdge(self):
 
 	def testScrollColumnsRegular(self):
 		# assume 3 keys for navigate columns
-		t = SpritesTable(DummyTable(6, 4), 1, 5, 3, True)
+		t = SpritesTable(1, 6, 4, 5, 3, True)
 		# cannot scroll to previous at start
 		self.assertFalse(t.scroll(direction=1, keyIdx=0))
 		self.assertTrue(t.scroll(direction=1, keyIdx=2))
@@ -94,7 +87,7 @@ def testScrollColumnsRegular(self):
 
 	def testScrollColumnsEdge(self):
 		# test table with 6 columns
-		t = SpritesTable(DummyTable(6, 6), 1, 5, 3, True)
+		t = SpritesTable(1, 6, 6, 5, 3, True)
 		# cannot scroll to previous at start
 		self.assertFalse(t.scroll(direction=1, keyIdx=0))
 		self.assertTrue(t.scroll(direction=1, keyIdx=2))
@@ -107,7 +100,7 @@ def testScrollColumnsEdge(self):
 		self.assertFalse(t.scroll(direction=1, keyIdx=0))
 
 	def testNextResult(self):
-		t = SpritesTable(DummyTable(4, 5), 1, 5, 13, True)
+		t = SpritesTable(1, 4, 5, 5, 13, True)
 		t.searchResults = [(1, 1), (2, 3), (3, 3), (3, 4), (3, 5)]
 		# go through all results using next
 		self.assertTrue(t.nextResult())
@@ -129,7 +122,7 @@ def testNextResult(self):
 		self.assertEqual(t.getMappedColumns(), [5])
 
 	def testPrevResult(self):
-		t = SpritesTable(DummyTable(4, 5), 1, 5, 13, True)
+		t = SpritesTable(1, 4, 5, 5, 13, True)
 		t.searchResults = [(1, 1), (2, 3), (3, 3), (3, 4), (3, 5)]
 		t.resultIdx = 4
 		t.onSearchResult = True
@@ -154,7 +147,7 @@ def testPrevResult(self):
 		self.assertEqual(t.getCurrPosition(), (1, 1))
 
 	def testGetOccurrences(self):
-		t = SpritesTable(DummyTable(4, 5), 1, 5, 13, True)
+		t = SpritesTable(1, 4, 5, 5, 13, True)
 		t.searchResults = [(1, 1), (2, 3), (3, 3), (3, 4), (3, 5)]
 		# start off at (1, 1)
 		self.assertEqual(t.getOccurrences(), (0, 0))
@@ -172,7 +165,7 @@ def testGetOccurrences(self):
 		self.assertEqual(t.getOccurrences(), (0, 0))
 
 	def testFilters(self):
-		t = SpritesTable(DummyTable(4, 5), 1, 5, 13, True)
+		t = SpritesTable(1, 4, 5, 5, 13, True)
 		t.searchResults = [(1, 1), (2, 3), (3, 3), (3, 4), (3, 5)]
 		t.filterRows = True
 		t.checkAndApplyFilters()
@@ -195,7 +188,7 @@ def testFilters(self):
 		self.assertEqual(t.columns, [1, 2, 3, 4, 5])
 
 	def testExpandTable(self):
-		t = SpritesTable(DummyTable(8, 10), 1, 5, 4, True)
+		t = SpritesTable(1, 8, 10, 5, 4, True)
 		t.searchResults = [(2, 2), (4, 7), (6, 3)]
 		t.filterRows = True
 		t.resultIdx = 0

addon/manifest.ini

@@ -3,7 +3,7 @@ summary = "Sprites table navigation for Chrome"
 description = """This addon introduces a non-linear navigation mode called SPRITEs to navigate tables on webpages browsed using google Chrome."""
 author = "Venkatesh Potluri <[email protected]>"
 url = https://make4all.github.io/sprites/tutorial/tutorial.html
-version = 0.9
+version = 0.9.1
 docFileName = readme.html
 minimumNVDAVersion = 2020.3
 lastTestedNVDAVersion = 2021.1

buildVars.py

@@ -25,7 +25,7 @@ def _(arg):
 	# Translators: Long description to be shown for this add-on on add-on information from add-ons manager
 	"addon_description": _("""This addon introduces a non-linear navigation mode called SPRITEs to navigate tables on webpages browsed using google Chrome."""),
 	# version
-	"addon_version": "0.9",
+	"addon_version": "0.9.1",
 	# Author(s)
 	"addon_author": "Venkatesh Potluri <[email protected]>",
 	# URL for the add-on documentation support