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

More column sizing adjustments #646

Merged
merged 7 commits into from
Jan 10, 2024
Merged

More column sizing adjustments #646

merged 7 commits into from
Jan 10, 2024

Conversation

tmiw
Copy link
Collaborator

@tmiw tmiw commented Jan 8, 2024

This PR performs the following to address feedback posted at #644 (comment):

  1. Limits displayed grid squares to six characters in length and actually ignores invalid input (vs. trying to calculate a distance that's probably wrong).
  2. Instead of using wxWidgets' column autosizing, implement our own, ensuring that column sizes don't drop below hardcoded minimums.

@tmiw
Copy link
Collaborator Author

tmiw commented Jan 8, 2024

@barjac, hopefully this works for you?

@Tyrbiter
Copy link

Tyrbiter commented Jan 8, 2024

I built and installed this latest version. There are some issues.

The 2 Mode columns are different widths for some reason. The SNR column is a bit too wide. The Locator column is now wider than the new maximum 6 characters displayed. The distance column could be narrowed a little, 5 digits should be sufficient.

I appreciate how difficult this is, it may also be affected by font sizes.

@barjac
Copy link

barjac commented Jan 8, 2024

Yes, here is a direct comparison of before this change and now:
Screenshot_20240106_224900c
Screenshot_20240108_142420
Screenshot_20240108_182129

@tmiw
Copy link
Collaborator Author

tmiw commented Jan 8, 2024

I fixed the Mode sizing discrepancy and added a change to the sizing calculation that might help. Unfortunately some space does need to be left for the sort indicator, so there might be a limit to how much further the other columns can shrink. For example, when sorting by SNR:

Screenshot 2024-01-08 at 9 16 31 AM

@Tyrbiter
Copy link

Tyrbiter commented Jan 8, 2024

This seems much better, and apologies for forgetting about the space used by the column sorting indicator.

I don't think much improvement is possible now.

@barjac
Copy link

barjac commented Jan 8, 2024

I agree, much improved :)
I have added another screen shot to the group above.
The locator 6 char limiting with case conversion is also working fine. One of our group has upper case last 2 and I noticed the correction this afternoon.

While on the looks of the GUI, could a 'dark mode' be made available from within the program? I can use the system theme in plasma5 on x86_64 but changing the theme in LXQT DE on the RPi4 does not affect FreeDV. Maybe I just can't find the correct setting.

@tmiw
Copy link
Collaborator Author

tmiw commented Jan 9, 2024

While on the looks of the GUI, could a 'dark mode' be made available from within the program? I can use the system theme in plasma5 on x86_64 but changing the theme in LXQT DE on the RPi4 does not affect FreeDV. Maybe I just can't find the correct setting.

If you're using wxWidgets 3.2, it should automatically detect the OS/dev environment's setting for that. Seems to on Ubuntu with GNOME, anyway, albeit not 100% perfect. For example, when starting in dark mode:

Screenshot 2024-01-08 at 11 07 43 PM

and then switching to light mode in the GNOME settings:

Screenshot 2024-01-08 at 11 07 54 PM

@tmiw
Copy link
Collaborator Author

tmiw commented Jan 9, 2024

BTW, I just checked in a change that should make dark->light mode transitions work a bit better in the FreeDV Reporter window (at least on Linux and maybe macOS; Windows does have some separate logic for that control in wxWidgets so it might not have had the problem in the first place).

@tmiw tmiw merged commit 09b1e52 into master Jan 10, 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.

3 participants