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 (its 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. Id like to request someone else (one or two) to
help me review this add-on. Thanks.
Cheers,
Joseph
