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

Status message improvements #632

Merged
merged 6 commits into from Dec 30, 2023
Merged

Status message improvements #632

merged 6 commits into from Dec 30, 2023

Conversation

tmiw
Copy link
Collaborator

@tmiw tmiw commented Dec 29, 2023

This PR adds several improvements to the recently-added status message feature:

  1. Allows user to refresh the status update (and cause it to highlight to all users) by pushing the Set button. (1.9.6 previously disabled this button unless the message was actually changing.)
  2. Increases the priority of the message highlight color so that it takes precedence over all other highlight types (i.e. updating the message while transmitting or receiving will cause a purple highlight before returning to RX/TX highlighting).

@barjac
Copy link

barjac commented Dec 29, 2023

Hi,
Tested in x86_64 and aarch64 @ #a7198 and it's looking good :)

I can't test purple replacing blue but it replaces red just fine.

One issue. The Msg column is again not resizing. This time the Version column is not affected, only Msg. I'm now wondering if this is a feature not a bug?

Edit: Ah, it is a bug, as the Msg column does return to narrow after a full FreeDV restart. Re-starting only the reporter retains the wide column.

To reproduce,
Start FreeDV
Add a short 'QRL' message.
Change message to '0123456789abcde'
Change message back to 'QRL'

@tmiw
Copy link
Collaborator Author

tmiw commented Dec 29, 2023

One issue. The Msg column is again not resizing. This time the Version column is not affected, only Msg. I'm now wondering if this is a feature not a bug?

Edit: Ah, it is a bug, as the Msg column does return to narrow after a full FreeDV restart. Re-starting only the reporter retains the wide column.

To reproduce, Start FreeDV Add a short 'QRL' message. Change message to '0123456789abcde' Change message back to 'QRL'

It looks like wxWidgets purposely doesn't shrink columns unless everything is deleted first. I checked in a change that will allow you to at least reset the column widths if you change the band filter. I don't think we can do much more unless wxWidgets itself changes.

@barjac
Copy link

barjac commented Dec 29, 2023

It looks like wxWidgets purposely doesn't shrink columns unless everything is deleted first. I checked in a change that will allow you to at least reset the column widths if you change the band filter. I don't think we can do much more unless wxWidgets itself changes.

IIRC This was working fine until the last changes to allow the colour highlight to change from red or blue to purple.

@tmiw
Copy link
Collaborator Author

tmiw commented Dec 29, 2023

It looks like wxWidgets purposely doesn't shrink columns unless everything is deleted first. I checked in a change that will allow you to at least reset the column widths if you change the band filter. I don't think we can do much more unless wxWidgets itself changes.

IIRC This was working fine until the last changes to allow the colour highlight to change from red or blue to purple.

I'm not sure how considering that the only sizing related change was the very last commit to fix it for the band filter change case. Can you confirm by running master / 1.9.6 just to be sure?

@barjac
Copy link

barjac commented Dec 29, 2023

I just rebuilt 1.9.6 #c7c97 to test and you are correct.

It's a subtle difference. I was testing previously in 1.9.6 #c7c97 by clicking 'Track' to move to a filter with some wide messages (All) and my own which was narrow. In this case it still works fine in the current branch, the Msg column changes width correctly.

If I do as above to show this current issue without switching filter and click Track or Band or Freq while the message column is wide and the message 'QRL' is narrow the column width does in fact reduce to fit.

So I'm guessing that whatever happens on a filter change is needed on a message change too?

Even just clicking on freq (even though that is already selected) causes the Msg column to resize correctly.

@tmiw
Copy link
Collaborator Author

tmiw commented Dec 30, 2023

So I'm guessing that whatever happens on a filter change is needed on a message change too?

Unfortunately with the current wxWidgets library, that means deleting all the rows from the list, then re-adding them individually. Here's how SetColumnWidth is defined:

    if ( width == wxLIST_AUTOSIZE_USEHEADER || width == wxLIST_AUTOSIZE )
    {
        wxListCtrlMaxWidthCalculator calculator(this, col);

        calculator.UpdateWithWidth(AUTOSIZE_COL_MARGIN);

        if ( width == wxLIST_AUTOSIZE_USEHEADER )
            calculator.UpdateWithWidth(ComputeMinHeaderWidth(column));

        //  if the cached column width isn't valid then recalculate it
        wxColWidthInfo* const pWidthInfo = m_aColWidths.Item(col);
        if ( pWidthInfo->bNeedsUpdate )
        {

bNeedsUpdate seems to get set to true only if the new width is longer than the existing one (i.e. when modifying or adding rows), using similar logic to the below:

        // calculate the width of the item and adjust the max column width
        wxColWidthInfo *pWidthInfo = m_aColWidths.Item(col);
        int width = GetItemWidthWithImage(&item);
        item.SetWidth(width);
        if (width > pWidthInfo->nMaxWidth)
        {
            pWidthInfo->nMaxWidth = width;
            pWidthInfo->bNeedsUpdate = true;
        }

Anyway, I don't think doing a full erase and re-add (especially as common as it would likely end up being given that people can re-trigger reporting of existing messages) is going to look all that great for users, either. I'll merge this for the time being and we can revisit if wxWidgets ever gives us the ability to actually resize columns smaller.

@tmiw tmiw merged commit deb58f3 into master Dec 30, 2023
2 checks passed
@barjac
Copy link

barjac commented Dec 30, 2023

OK, I agree that it does cause a full re-write of the table which is visible to the user, whether any would actually notice is debatable ;)
I can always just click whichever is already in use of the Band or Freq buttons to clean up the table manually as a workaround.

@barjac
Copy link

barjac commented Dec 30, 2023

Ah! I have just noticed a related issue doing some more testing in a QSO with another station.

Using any of the Band filters and 'Track' with 'Band' selected all is OK, but with 'Freq' selected the whole table is refreshing (visibly the colours and text momentarily flashes) on every 'Last Update' change by any station, transmitting or receiving. It is more noticeable with colours on the screen.

I noticed it in this branch, but I am building master now to double check in Mageia 9 stable, rather than Cauldron (dev).

Edit:
Yes it is there in master @ #a8f58 in Mageia 9 stable. The flicker is about 100-200ms before the seconds change in any 'Last Update' line but only when 'Track' and 'Freq' are selected.

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.

None yet

2 participants