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 case sensitivity filter option in NVDA Find dialog box #4584
Comments
Comment 1 by jteh on 2014-10-28 21:12 For anyone interested in implementing this, the TextInfo.find method (which is used to do the actual search) already supports case sensitivity. Therefore, the task here is to implement a custom dialog for the Find dialog; see cursorManager.CursorManager.doFindTextDialog. We currently use wx.TextEntryDialog, but we can't add additional options with that. |
Comment 2 by nvdakor on 2014-10-28 23:19
|
Comment 3 by nvdakor on 2014-10-28 23:26 |
Comment 4 by jteh on 2014-10-29 01:25 We probably need to create a subclassable singleton dialog, as there are several cases where this is needed now. I'd probably just leave the dialog in cursorManager. |
Comment 5 by nvdakor (in reply to comment 4) on 2014-10-29 05:49
I'm looking at various dialogs to see which one might be the "template" for the new find dialog (welcome dialog seems promising). A new dialog from scratch is indeed in order.
If so, can you do it in GUI/init so add-ons can also benefit from this refactor?
I agree. This means two things: a FindDialog will be created in CursorManager module, which means CursorManager.doFindDialog is no longer needed, as all the scripts need to do is call FindDialog.run. As for passing the new find text, that can be done from the new dialog, with onOK calling CursorManager.doFind (this guy may need caseSensitive argument too). |
Comment 6 by blindbhavya on 2014-10-29 06:20 |
Comment 7 by nvdakor on 2014-10-29 10:52
|
Comment 8 by blindbhavya on 2014-10-30 08:10 |
Comment 9 by blindbhavya on 2014-10-30 08:14 |
Comment 10 by nvdakor (in reply to comment 8) on 2014-10-30 08:18
For now, I've titled it that way to let people try both implementations (I'm working on this from a separate branch). At the moment, it's a proof of concept which may need some further touches from other devs before it can be tried out by others.
As for match whole word, I'm personally against adding this, as the find facility is meant to search for anything (whole word, part of a word, etc.). Case sensiticity might be something to consider (after all, this is what the ticket is about), but not match whole word.
|
Comment 11 by nvdakor on 2014-10-30 08:22 |
Comment 12 by jteh (in reply to comment 7) on 2014-10-30 09:29 Replying to nvdakor:
IMO, this sort of thing isn't necessary. You're working in a branch, so there's no need to distinguish because the fact that it's a branch distinguishes it, if that makes sense. Also, you want to avoid unnecessary effort when you come to finalise your work.
Again, I think this is unnecessary. Case sensitivity is just an option; users don't need to know about that before they even use the command. That's why we have a What's New. :) |
Comment 13 by Joseph Lee <joseph.lee22590@... on 2014-10-30 20:01
|
Comment 14 by Joseph Lee <joseph.lee22590@... on 2014-10-30 20:01
|
Comment 15 by nvdakor (in reply to comment 12) on 2014-10-30 20:03
Fixed in the commit prior to the latest commit.
Got it - also done. |
Comment 16 by jteh on 2014-11-12 01:29 Re the docstring for FindDialog: I don't think this should mention virtual buffers, document review, etc., as it applies to all CursorManagers and will only work with CursorManagers. Perhaps it could just say "A dialog used to specify text to find in a cursor manager." Right now, you have two vertical sizers inside mainSizer: one with the options and the other with the buttons. Instead, I'd put the find label and text control in a horizontal sizer and then add that, the check box and the buttons sizer to mainSizer. Reasons:
In onOk, you call Destroy before doing anything else. That's not safe, since the controls could theoretically be destroyed before you get their values. Call Destroy at the end. I'd move all of the code from doFindTextDialog into script_find, especially given how short it is now. The second method is redundant. I suspect we didn't do that initially because GUI stuff used to be a bit more complicated than it is now. Thanks. |
Comment 17 by nvdakor (in reply to comment 16) on 2014-11-12 04:23
Done.
Trying to fix this - my code produces overlapping controls, so I'm working on how to prevent this from happening.
Also done.
I will do so - I do agree that it is better to eliminate redundant methods now that the code is much simpler. |
Comment 18 by jteh on 2014-11-12 04:56 |
Comment 19 by Joseph Lee <joseph.lee22590@... on 2014-11-12 05:23
|
Comment 20 by nvdakor (in reply to comment 18) on 2014-11-12 05:33
Done. Thanks for reminding me. |
Comment 21 by blindbhavya on 2014-11-12 16:10 |
Comment 22 by nvdakor (in reply to comment 21) on 2014-11-12 16:34
I worked on the second option. There was no need to add a separate case sensitive find dialog. Adding the functionality required more than just creating the dialog (see the commit comments). Thanks. |
Comment 23 by blindbhavya on 2014-11-12 16:36 |
Comment 24 by James Teh <jamie@... on 2014-11-19 04:59
Changes:
|
Comment 25 by jteh on 2014-11-19 05:01 |
Comment 26 by James Teh <jamie@... on 2014-12-03 04:44
Changes:
|
Comment 27 by jteh on 2014-12-03 04:47 |
Comment 28 by twynn92 on 2015-01-09 08:38
Results: On subsequent access to the search function, the "Case sensitive" check-box's state is not remembered and reverts to the unchecked state. Expected results: In step five, the latter case should still remember the "Case sensitive" check-box's status. As for the former, I am not sure as it may be expected that the user is performing a new search entirely. This state should also be remembered for Shift+NVDA+F3 as well. Random thought: Maybe a radio-button should be added for search direction? This should be a new ticket I am assuming, or the summary should be modified. |
Comment 29 by nvdakor on 2015-01-09 08:48 |
…ing NVDA's find dialog. For Example, if Food is searched for with case sensitivity on Food will be found when pressing NVDA+f3, but food will not.
Reported by blindbhavya on 2014-10-28 14:15
The summary explains it all, and probably a use case isn't required.
Thanks.
The text was updated successfully, but these errors were encountered: