Smartcraft-Media-Tech/ntfysh-windows
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases 4 Wiki Activity

Add Publishing #7 #16

Open
seba76 wants to merge 2 commits from seba76/master into master
pull from: seba76/master
merge into: :master
Conversation 1 Commits 2 Files Changed 8
+692 -240
seba76 commented 2024-12-17 17:02:20 -05:00 (Migrated from github.com)
Copy Link
Copy Source

I've added simple dialog where you can sent message title and message text which you can use to publish notification. It is invoked by double clicking on topic or right-click and selecting menu item in Main form.
image

I've added simple dialog where you can sent message title and message text which you can use to publish notification. It is invoked by double clicking on topic or right-click and selecting menu item in Main form. ![image](https://github.com/user-attachments/assets/b1d8e89e-be60-4deb-bca9-f1f4f6f10b3e)
alexhorner (Migrated from github.com) requested changes 2024-12-17 17:33:32 -05:00
alexhorner (Migrated from github.com) left a comment
Copy Link
Copy Source

Hi there @seba76

I have provided a set of changes for you to review. Please don't hesitate to ask if you need further clarification on anything I have said.

Thanks for your contiribution!

Hi there @seba76 I have provided a set of changes for you to review. Please don't hesitate to ask if you need further clarification on anything I have said. Thanks for your contiribution!
ntfysh_client/MainForm.Designer.cs
@@ -147,0 +157,4 @@
fileToolStripMenuItem.DropDownItems.AddRange(new System.Windows.Forms.ToolStripItem[] { exitToolStripMenuItem1, settingsToolStripMenuItem });
fileToolStripMenuItem.Name = "fileToolStripMenuItem";
fileToolStripMenuItem.Size = new System.Drawing.Size(37, 20);
fileToolStripMenuItem.Text = "File";
alexhorner (Migrated from github.com) commented 2024-12-17 17:32:01 -05:00
Copy Link
Copy Source

Nice-to-have: It'd be good if an icon was found from the VS Icon Pack and placed against this menu item the same as all other menu items

Nice-to-have: It'd be good if an icon was found from the VS Icon Pack and placed against this menu item the same as all other menu items
ntfysh_client/MainForm.cs
alexhorner (Migrated from github.com) commented 2024-12-17 17:11:50 -05:00
Copy Link
Copy Source

The rest of the project uses explicit type declarations instead of var. For consistency, please could you update all instances of var with their explicit types?

The rest of the project uses explicit type declarations instead of `var`. For consistency, please could you update all instances of `var` with their explicit types?
alexhorner (Migrated from github.com) commented 2024-12-17 17:17:30 -05:00
Copy Link
Copy Source

It appears with the null conditional ? that topicAndHost could be null resulting in a NullReferenceException in the following two lines should there be an issue. Please could you verify whether ToString()can return null, and if so, handle the condition with an error dialog box accordingly?

It appears with the null conditional ? that `topicAndHost` could be null resulting in a `NullReferenceException` in the following two lines should there be an issue. Please could you verify whether `ToString()`can return null, and if so, handle the condition with an error dialog box accordingly?
alexhorner (Migrated from github.com) commented 2024-12-17 17:26:32 -05:00
Copy Link
Copy Source

Please handle the return value and return an error dialog box if we don't get a success response

Please handle the return value and return an error dialog box if we don't get a success response
ntfysh_client/Notifications/NotificationListener.cs
alexhorner (Migrated from github.com) commented 2024-12-17 17:29:07 -05:00
Copy Link
Copy Source

This doesn't appear to handle credentials on a topic. You can retrieve the topic from SubscribedTopicsByUnique within this class

This doesn't appear to handle credentials on a topic. You can retrieve the topic from `SubscribedTopicsByUnique` within this class
ntfysh_client/SendMessageForm.Designer.cs
@@ -0,0 +1,108 @@
namespace ntfysh_client
{
partial class SendMessageForm
alexhorner (Migrated from github.com) commented 2024-12-17 17:24:49 -05:00
Copy Link
Copy Source

Please add a title to the window so that it doesn't just say "SendMessageForm" in the title bar

Please add a title to the window so that it doesn't just say "SendMessageForm" in the title bar
This pull request has changes conflicting with the target branch.
  • ntfysh_client/MainForm.cs
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin seba76/master:seba76/master
git checkout seba76/master
Sign in to join this conversation.
Reviewers
No Reviewers
alexhorner
Labels
Clear labels
bug
Something isn't working
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 question
Further information is requested
 wontfix
This will not be worked on
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
minster586 (minster586)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Smartcraft-Media-Tech/ntfysh-windows#16