[nvda-addons] Re: ReadFeeds 3.0-dev review results

  • From: "Joseph Lee" <joseph.lee22590@xxxxxxxxx>
  • To: <nvda-addons@xxxxxxxxxxxxx>
  • Date: Tue, 6 Dec 2016 04:50:54 -0800

Hi,
Yep, something we need to talk to Reef (NV Access) about this for 2017.
Cheers,
Joseph

-----Original Message-----
From: nvda-addons-bounce@xxxxxxxxxxxxx
[mailto:nvda-addons-bounce@xxxxxxxxxxxxx] On Behalf Of Noelia
Sent: Tuesday, December 6, 2016 4:46 AM
To: nvda-addons@xxxxxxxxxxxxx
Subject: [nvda-addons] Re: ReadFeeds 3.0-dev review results

Hi, I have used addLabeledControl with sHelper. Hope now it's right.
I get this when trying to use addItem with sHelper, I don't know why:

   File "gui\guiHelper.pyo", line 256, in addItem
NotImplementedError: Use addLabeledControl instead


Please test and, if it's OK, I will release the development version when
it's greenlight.
I think we want to go far, not fast (it's a spanish expression, I don't know
if it's said in English :) ).
Cheers.

El 06/12/2016 a las 13:18, Joseph Lee escribió:

Hi,
Wx.BoxSizer natively does not have addItem function - the new 
guiHelper.BoxSizerHelper does.
Cheers,
Joseph

-----Original Message-----
From: nvda-addons-bounce@xxxxxxxxxxxxx 
[mailto:nvda-addons-bounce@xxxxxxxxxxxxx] On Behalf Of Noelia
Sent: Monday, December 5, 2016 9:03 PM
To: nvda-addons@xxxxxxxxxxxxx
Subject: [nvda-addons] Re: ReadFeeds 3.0-dev review results

Hi Joseph and all, thanks for the review.
I have set a default file in the configuration, and prevent from 
rename or delete it, as well as prevent from do this with the feed set as
the default.
Thanks a lot for trying to get quality in add-ons. I saw the key 
error, and thought that the default file could be deleted, but it's 
better your suggestion.
The problem could come if someone set a feed as the default, reinstall 
the add-on and don't update it properly. This could be fixed easily 
setting another feed as default.
About the gui, unfortunately box sizers don't support addItem or 
addLabeledControl.
I think it will be needed to remove box sizers, though I didn't want 
to do it to preserve the config profiles dialog appearance if possible.
Feed-back is welcome and hope more reviewers join this work.
Here is the log message about sizers. Please let me know if I'm 
missing
something:
AttributeError: 'BoxSizer' object has no attribute 'addItem'
The enhancedgui branch is updated on Bitbucket and GitHub.
Cheers.

El 06/12/2016 a las 0:29, Joseph Lee escribió:
Hi Noelia and others,



Basic review results for ReadFeeds 3.0-dev:



·         License and copyright: pass

·         User experience: partial

·         Security: pass



A potential showstopper bug found: if you are trying to open Feeds 
dialog for the first time, Config says, “KeyError: ‘addressFile’”. 
You can open this dialog, but the elements in that dialog would not 
be aligned correctly. I suggest catching this case and use default 
config for this.



Code wise: is filterLabeledCtrl used anywhere besides getting the 
edit field sizer? If not, you can either add the control to the 
managing sizer directly 
(sHelper/whateverSizer.addItem/addLabeledControl(self,
label, wx.TextCTRL), or use whateverTextCtrl.control. Personally, I 
prefer to add the label constant directly when adding controls 
instead of using a separate variable to hold control labels (it’s up 
to you, but NVDA Core uses the label variable approach for readability).



A blinking green light due to the possible showstopper bug, otherwise 
looks fine to me. I’d like to request someone else (one or two) to 
help me review this add-on. Thanks.

Cheers,

Joseph

----------------------------------------------------------------
NVDA add-ons: A list to discuss add-on code enhancements and for 
reporting bugs.

Community addons are available from: http://addons.nvda-project.org To ;
send a message to the list: nvda-addons@xxxxxxxxxxxxx To change your 
list
settings/unsubscribe: //www.freelists.org/list/nvda-addons
To contact list moderators: nvda-addons-moderators@xxxxxxxxxxxxx

----------------------------------------------------------------
NVDA add-ons: A list to discuss add-on code enhancements and for reporting
bugs.

Community addons are available from: http://addons.nvda-project.org To ;
send a message to the list: nvda-addons@xxxxxxxxxxxxx To change your 
list settings/unsubscribe: //www.freelists.org/list/nvda-addons
To contact list moderators: nvda-addons-moderators@xxxxxxxxxxxxx

----------------------------------------------------------------
NVDA add-ons: A list to discuss add-on code enhancements and for reporting
bugs. 

Community addons are available from: http://addons.nvda-project.org To send
a message to the list: nvda-addons@xxxxxxxxxxxxx To change your list
settings/unsubscribe: //www.freelists.org/list/nvda-addons
To contact list moderators: nvda-addons-moderators@xxxxxxxxxxxxx

----------------------------------------------------------------
NVDA add-ons: A list to discuss add-on code enhancements and for reporting bugs.

Community addons are available from: http://addons.nvda-project.org
To send a message to the list: nvda-addons@xxxxxxxxxxxxx
To change your list settings/unsubscribe: 
//www.freelists.org/list/nvda-addons
To contact list moderators: nvda-addons-moderators@xxxxxxxxxxxxx

Other related posts: