Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FreeDV Reporter message behavior changes #647

Merged
merged 11 commits into from
Jan 14, 2024
Merged

Conversation

tmiw
Copy link
Collaborator

@tmiw tmiw commented Jan 10, 2024

Resolves #645 by changing FreeDV Reporter message behavior as follows:

  1. Left-click of the Set button no longer saves the message on the server.
  2. Right-click of the Set button brings up a new option to save the message (along with sending it to the server).

Additionally, tooltips were added to each button in the FreeDV Reporter window to ensure that users can find the expected behavior of each.

tmiw added 2 commits January 9, 2024 22:42
1. Left-click of the Set button no longer saves the message on the server.
2. Right-click of the Set button brings up a new option to save the message (along with sending it to the server).

Additionally, tooltips were added to each button in the FreeDV Reporter window to ensure that users can find
the expected behavior of each.
@barjac
Copy link

barjac commented Jan 10, 2024

Resolves #645 by changing FreeDV Reporter message behavior as follows:

1. Left-click of the Set button no longer saves the message on the server.

I assume that is a typo?
Did you mean that left-click sends the message to the server but does not save it to the local list?
Edit: Ah OK I see what you were trying to say! s/on the/sent to/ I was reading it differently :\

2. Right-click of the Set button brings up a new option to save the message (along with sending it to the server).

I don't really see a need for a context menu, can right-click not simply send the message to the server and save it to the list?

Additionally, tooltips were added to each button in the FreeDV Reporter window to ensure that users can find the expected behavior of each.

That sounds good - I have not tested any of this yet though.

I think a GUI means of removing messages from the list is needed.
Edit: Maybe a pick list from the 'Clear' button (same as the Message list) to remove an item from the list? Possibly with confirmation?

@Tyrbiter
Copy link

Tyrbiter commented Jan 10, 2024

Just built and installed this, the only problem is that the Set and Save button is appearing at the top left of the reporter window instead of next to the Set button. Other than that, it seems to do what it says.

And @barjac has suggested a remove option on the Clear button, that does make sense to me as well.

@tmiw
Copy link
Collaborator Author

tmiw commented Jan 10, 2024

I don't really see a need for a context menu, can right-click not simply send the message to the server and save it to the list?

The context menu is mainly to help people see what will happen if you right-click. Otherwise, the only way someone would know that right-click == save would be to read the tooltip (which I'm not sure is visible enough). It also gives us flexibility to add other features in the future if we wanted.

I think a GUI means of removing messages from the list is needed. Edit: Maybe a pick list from the 'Clear' button (same as the Message list) to remove an item from the list? Possibly with confirmation?

Maybe another right-click menu for the Clear button that has "Remove selected from list" and "Remove all from list" options?

Just built and installed this, the only problem is that the Set and Save button is appearing at the top left of the reporter window instead of next to the Set button. Other than that, it seems to do what it says.

This should be fixed now.

@Tyrbiter
Copy link

Tyrbiter commented Jan 10, 2024

Maybe another right-click menu for the Clear button that has "Remove selected from list" and "Remove all from list" options?

Yes, that would do it for me as long as the clear all entry is not the default selection.

Just built and installed this, the only problem is that the Set and Save button is appearing at the top left of the reporter window instead of next to the Set button. Other than that, it seems to do what it says.

This should be fixed now.

Thanks. It's in the right place now.

@tmiw
Copy link
Collaborator Author

tmiw commented Jan 11, 2024

Yes, that would do it for me as long as the clear all entry is not the default selection.

Added a right-click context menu for the Clear button as well with "clear selected" and "clear all" items.

@barjac
Copy link

barjac commented Jan 11, 2024

I don't like this, sorry. :(
I was expecting the right click on 'Clear' to display the list of messages currently saved to allow one to be selected and deleted totally independently of any message entry in use.

Consider this scenario, I am in a QSO and decide to remove an unimportant message to replace it with another I want to use.

With the implementation as it is now I would have to send the message (that I am about to delete) to the server and make it current and world visible before I could delete it, unless I am mistaken?

For this to work it would need to have the 'click to select and send' removed so that a message could be selected and deleted without automatically sending it. This would then require an extra click on 'Set' when using the list.

This currently only works satisfactorily if the modem is stopped, which is never the case while operating FreeDV.

@tmiw
Copy link
Collaborator Author

tmiw commented Jan 11, 2024

For this to work it would need to have the 'click to select and send' removed so that a message could be selected and deleted without automatically sending it. This would then require an extra click on 'Set' when using the list.

If that's okay with you, I can remove that behavior and require users to explicitly click Set to send the message.

@barjac
Copy link

barjac commented Jan 11, 2024

For this to work it would need to have the 'click to select and send' removed so that a message could be selected and deleted without automatically sending it. This would then require an extra click on 'Set' when using the list.

If that's okay with you, I can remove that behavior and require users to explicitly click Set to send the message.

I don't think it's ideal but it is acceptable and I appreciate it's a lot less work :)

That would have the advantage that the message selected would be transferred to the 'Message' box and could be edited before sending :)

BTW I don't like 'Set' as it's not very meaningful. I think 'Send' would be more acceptable.

@tmiw
Copy link
Collaborator Author

tmiw commented Jan 12, 2024

I don't think it's ideal but it is acceptable and I appreciate it's a lot less work :)

That would have the advantage that the message selected would be transferred to the 'Message' box and could be edited before sending :)

Done.

BTW I don't like 'Set' as it's not very meaningful. I think 'Send' would be more acceptable.

Also changed :)

@barjac
Copy link

barjac commented Jan 14, 2024

Looks fine now to me - everything works as expected - Well done!
I don't understand why Americans appear to think that the Msg field is for QTH though. Very strange. :)

@Tyrbiter
Copy link

I don't understand why Americans appear to think that the Msg field is for QTH though. Very strange. :)

Maybe adding a bearing column would avoid this ;-) The US is a big place so beam headings can be almost any direction even on HF.

@tmiw tmiw merged commit ebad7ce into master Jan 14, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minor enhancement to reporter message entry - discussion
3 participants