Add ability to view new add-ons #16728
Conversation
WalkthroughThe changes introduce functionality to filter, notify, and manage new add-ons in an NVDA add-on store. Users can now select reset intervals for new add-ons (monthly, weekly, at startup) and manage visibility of new add-ons through various filters. The application was enhanced with methods and entities related to caching, filtering, and displaying new add-ons. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant DataManager
participant Config
User->>UI: Opens Add-on Store
UI->>UI: Display add-ons
UI->>Config: Fetch reset interval for new add-ons
Config-->>UI: Return reset interval (e.g., monthly)
UI->>DataManager: Request new add-ons based on interval
DataManager-->>UI: Return list of new add-ons
UI->>UI: Update display with new add-ons filter
User->>UI: Select new add-ons filter
UI->>DataManager: Filter new add-ons
DataManager-->>UI: Return filtered add-ons
UI->>UI: Update display with filtered new add-ons
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
See test results for failed build of commit f54b3667a1 |
… versions for existing add-ons
Co-authored-by: Luke Davis <[email protected]>
Co-authored-by: Luke Davis <[email protected]>
|
Hi @XLTechie , I think I've addressed all your comments. I'll fix the schedule job to show new add-ons at startup this weekend. |
|
@XLTechie (%x) is not showing dates in the local format. I think we should try to use winKernel. I'll test it this weekend. |
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range and nitpick comments (9)
source/addonStore/dataManager.py (2)
172-186: Consider adding comments for clarity.The method logic is sound, but adding comments would improve readability and maintainability.
def _shouldCacheCompatibleAddonsBackup(self) -> bool: # Check if the backup file exists if not os.path.exists(self._cacheCompatibleOldFile): return True resetNewAddons = config.conf["addonStore"]["resetNewAddons"] # If reset is set to startup, always create a backup if resetNewAddons == "startup": return True lastBackupTime = os.path.getmtime(self._cacheCompatibleOldFile) lastBackupDate = datetime.fromtimestamp(lastBackupTime) nowDate = datetime.now() diffDate = nowDate - lastBackupDate # Check if the reset period has elapsed if resetNewAddons == "weekly" and diffDate.days >= 7: return True if resetNewAddons == "monthly" and diffDate.days >= 30: return True return False
188-205: Use locale-specific date formatting.Consider using locale-specific date formatting for better user experience.
- formattedLastBackupDate = lastBackupDate.strftime("%d/%m/%Y") + formattedLastBackupDate = lastBackupDate.strftime("%x")source/gui/addonStoreGui/controls/storeDialog.py (3)
199-211: Consider a more descriptive label text.The label text for the new filter control could be more descriptive to improve user understanding.
- labelText=pgettext("addonStore", "Release date filter:"), + labelText=pgettext("addonStore", "New add-ons filter:"),
344-349: Consider adding comments for clarity.The logic is sound, but adding comments would improve readability and maintainability.
def _toggleFilterControls(self): self.channelFilterCtrl.Clear() for c in _channelFilters: if c != Channel.EXTERNAL: self.channelFilterCtrl.Append(c.displayString) # Show or hide the new filter control based on the selected tab if self._storeVM._filteredStatusKey == _StatusFilterKey.AVAILABLE: self.newFilterCtrl.Enable() self.newFilterCtrl.Show() else: self.newFilterCtrl.Hide() self.newFilterCtrl.Disable()
410-417: Consider adding comments for clarity.The methods are well-implemented, but adding comments would improve readability and maintainability.
def onIncompatibleFilterChange(self, evt: wx.EVT_CHECKBOX): self._storeVM._filterIncludeIncompatible = self.includeIncompatibleCtrl.GetValue() # Show or hide the new filter control based on the incompatible filter if self._storeVM._filterIncludeIncompatible: self.newFilterCtrl.Hide() self.newFilterCtrl.Disable() else: self.newFilterCtrl.Enable() self.newFilterCtrl.Show() self._storeVM.refresh() def onNewFilterChange(self, evt: wx.EVT_CHOICE): index = self.newFilterCtrl.GetCurrentSelection() self._storeVM._filterNew = list(NewStatus)[index] self._storeVM.refresh()source/addonStore/models/status.py (2)
47-60: Add a docstring for clarity.Consider adding a docstring to the
NewStatusclass to improve readability and maintainability.class NewStatus(DisplayStringEnum): """ Enumeration representing the status of new add-ons in the add-on store. """ ALL = enum.auto() NEW = enum.auto() @property def _displayStringLabels(self) -> Dict["EnabledStatus", str]: resetNewAddonsDate = getResetNewAddonsDate() return { # Translators: The label of an option to filter the list of add-ons in the add-on store dialog. self.ALL: pgettext("addonStore", "All"), # Translators: The label of an option to filter the list of add-ons in the add-on store dialog. self.NEW: pgettext("addonStore", "New add-ons (%s)" % resetNewAddonsDate), }
Line range hint
362-386:
Add a docstring for clarity.Consider adding a docstring to the
_StatusFilterKeyclass to improve readability and maintainability.class _StatusFilterKey(DisplayStringEnum): """ Enumeration representing keys for filtering by status in the NVDA add-on store. """ INSTALLED = enum.auto() UPDATE = enum.auto() AVAILABLE = enum.auto() INCOMPATIBLE = enum.auto() @property def _displayStringLabels(self) -> Dict["_StatusFilterKey", str]: return { # Translators: The label of a tab to display installed add-ons in the add-on store. # Ensure the translation matches the label for the add-on list which includes an accelerator key. self.INSTALLED: pgettext("addonStore", "Installed add-ons"), # Translators: The label of a tab to display updatable add-ons in the add-on store. # Ensure the translation matches the label for the add-on list which includes an accelerator key. self.UPDATE: pgettext("addonStore", "Updatable add-ons"), # Translators: The label of a tab to display available add-ons in the add-on store. # Ensure the translation matches the label for the add-on list which includes an accelerator key. self.AVAILABLE: pgettext("addonStore", "Available add-ons"), # Translators: The label of a tab to display incompatible add-ons in the add-on store. # Ensure the translation matches the label for the add-on list which includes an accelerator key. self.INCOMPATIBLE: pgettext("addonStore", "Installed incompatible add-ons"), }source/gui/settingsDialogs.py (1)
3107-3130: Ensure consistency in naming conventions and code structure.The new settings for showing and resetting new add-ons are well-integrated. Ensure consistency in naming conventions and code structure for better readability and maintainability.
- showNewAddonsLabelText = _("&Show new add-ons:") + showNewAddonsLabelText = _("&Show New Add-ons:")user_docs/en/changes.md (1)
50-51: Improve clarity and adherence to style guide.Ensure each change log entry references an issue or pull request number and follows the style of one sentence per line.
- * You can check for new add-ons from the New available add-ons tab of the store. (#16681, @nvdaes) - * From the Add-on store settings panel, new options are available to determine if the presence of new add-ons should be notified at startup, and the period of time to reset new add-ons. + * You can check for new add-ons from the "New available add-ons" tab of the store. (#16681, @nvdaes) + * From the Add-on store settings panel, new options are available to determine if the presence of new add-ons should be notified at startup, and the period of time to reset new add-ons.
| resetNewAddons = config.conf["addonStore"]["resetNewAddons"] | ||
| if resetNewAddons == "startup": | ||
| # Translators: Message presented in the new add-ons tab, informing that new add-ons will be reset at startup | ||
| return _("Will be reset at startup") |
There was a problem hiding this comment.
@nvdaes you may have missed this one, or at least it's not showing as resolved at my end.
| timedeltaDays = 30 | ||
| else: # weekly | ||
| timedeltaDays = 7 | ||
| nextResetDate = lastBackupDate + timedelta(days=timedeltaDays) |
There was a problem hiding this comment.
@seanbudd how hard would it be to start embedding a generation timestamp in the JSON? I should think, not very.
source/addonStore/dataManager.py
Outdated
| return _("Will be reset at startup") | ||
| lastBackupTime = os.path.getmtime(self._cacheCompatibleOldFile) | ||
| lastBackupDate = datetime.fromtimestamp(lastBackupTime) | ||
| formattedLastBackupDate = lastBackupDate.strftime("%d/%m/%Y") |
There was a problem hiding this comment.
@nvdaes if it's only ever going to show, at most, 30 days, then the year isn't even necessary. In that case, using something like "%d %B" is, imho, more user friendly.
That said, if you're going to use a full date, even the AI agreed you should use "%x". You said it doesn't work, but I am curious how it fails, as %x has been standard for a long, long time.
On my system, %x returns 8/5, and in Spanish this is interpreted as May 8, not August 5. |
|
I'll test again with %x to show dates, since testing this in the Python console results are right. |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@XLTechie, in fact %x works as expected to format dates. The problem was that I was testing from source but with NVDA set to English. |
|
@coderabbitai resolve |
|
@coderabbitai full review |
|
@coderabbitai review |
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (1)
user_docs/en/userGuide.md (1)
2950-2963: Ensure clarity and consistency in new feature description.The added segment clearly describes the new feature for resetting the period of time to consider add-ons as new. However, the phrase "considered as new" could be simplified to "considered new" for better readability.
- Add-ons will be considered as new for a month (31 days) - Add-ons will be considered as new for a week (7 days) - Add-ons will be considered as new until NVDA is restarted + Add-ons will be considered new for a month (31 days) + Add-ons will be considered new for a week (7 days) + Add-ons will be considered new until NVDA is restarted
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@coderabbitai summary |
|
Luke wrote:
I understand that this would require to work in several repos, but, even if the release date is not presented in a column of the add-on list, I'd like to be timestamps added for the issue related to this PR. Now,the first time we start NVDA without cached old add-ons, the list is empty, and timestamps will make this cleaner. |
|
@nvdaes - thanks for investigating this approach. Given issues around the UX, we think it would be best to go with the solution of adding timestamps and the ability to sort by them. |
|
OK @seanbudd . I may create prs fr timestamps in the addon-datastore repo, in the addon-datastore-validation repo and in NVDA. |
Link to issue number:
Closes #16681
Summary of the issue:
List of available add-ons is very long, and some users may wish to show a list of recent add-ons with their description and other details to decide if they wish to install them.
Description of user facing changes
Description of development approach
Testing strategy:
Tested manually:
_cachedCompatibleAddons-old.jsonfile, saved when NVDA is started, and check that the second option of the Release date filter combo box shows that NVDA needs to be restarted to show new add-onsKnown issues with pull request:
None
Code Review Checklist:
Summary by CodeRabbit
New Features
NEWfor recently published add-ons, enhancing add-on discovery and management.Enhancements
Documentation