feat: Add Custom Notes feature for uninstaller entries#939
Conversation
Klocman
left a comment
There was a problem hiding this comment.
Functionally looks fine but there's some issues and oddities that prevent a merge.
I think the way the notes are implemented currently they have a very limited use since you cannot see the note or even know that the note exists by looking at the application list.
You have to know that there is a note for that specific item and manually open the note editor to read it, but if you remember there is a note you probably also remember what the note is saying.
I don't know how to resolve this, but as it is now it's unlikely to be used much. Maybe it could be shown on the left sidebar when selected, or in a tooltip on mouse hover. It could also show an extra note icon for the uninstaller entry, and hovering over that would show the tooltip? I'm open to discussion and different takes on this.
|
Pull from upstream to fix the failing check. |
on the app list you can see it at the very right side of things, which can be dragged if needed. I intentionally create a very bare minimum working feature without thinking of adding any special icons and other UI elements, so you can give me some inputs about how you do things around here, a little bit discussion and maybe a couple of commits later we are good to go. Tbh, I don't really know what I should add because this is my very first time contributing to any open-source project, so I decided to just took the description of the issue very literal |
|
thanks for all the feedback, I have read them all carefully, I will make some adjustment and get back to you dang this is exiciting |
e5e8478 to
516a0cd
Compare
Klocman
left a comment
There was a problem hiding this comment.
Good changes. I think the current column approach is good enough to merge.
The edit custom note item should be moved above Properties I think, It can be named simply "Edit note...".
Once this review is addressed it should be good to merge.
|
Suggestions to add for custom note, just pushing it a little bit further:
a must have for these 2 features is a HARD cap on how many apps can be selected at any given time. The maximum amount a user can select and edit, should not be more than 6. Because:
what do you think? if it aligns, I can create a new PR just for this 2 features. |
- Added `CustomNotesManager` to save and load notes persistently via XML (`CustomNotes.xml`). - Injected `CustomNote` property into `ApplicationUninstallerEntry` to bind notes directly to entries. - Created `CustomNoteDialog` UI to allow users to view and edit notes. - Added a "Custom Note" column to the main uninstaller list view. - Added an "Edit Custom Note..." option to the right-click context menu in the main window. - Fixed legacy MSTest compilation errors in `DrawingToolsTests` and `UninstallListTests` for .NET 8 compatibility. Resolves BCUninstaller#932
cde19a6 to
9a2ac53
Compare
|
all issue has been addressed, I also added a comment on each comment you made. I tried running the action on my fork, it passed let me know if you think any other changes should be made! |
Klocman
left a comment
There was a problem hiding this comment.
All good, thank you for the PR!
|
my pleasure! onto the next one |








CustomNotesManagerto save and load notes persistently via XML (CustomNotes.xml).CustomNoteproperty intoApplicationUninstallerEntryto bind notes directly to entries.CustomNoteDialogUI to allow users to view and edit notes.DrawingToolsTestsandUninstallListTestsfor .NET 8 compatibility.Resolves #932