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

  • From: Noelia <nrm1977@xxxxxxxxx>
  • To: nvda-addons@xxxxxxxxxxxxx
  • Date: Tue, 6 Dec 2016 14:08:34 +0100

OK, I will go ahead and release the development version of this add-on when NVDA 2016.4 is released.
I have fixed the issue when the text of the filter is not found. It required just to remove a line not needed.
Ps: In short, I will release development version for the following add-ons:
- clipContentsDesigner.
- placeMarkers.
- readFeeds.
- reportSymbols.

All them have passed basic review.
Thanks.



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

Hi,
Yes, works as advertised. Go ahead and let users test this one once 2016.4
is released.
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

----------------------------------------------------------------
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: