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

Make traffic and device windows display correctly #191

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

x0rloser
Copy link
Contributor

This PR fixes the below listed issues, which includes issue #132 from the issue tracker:
I have only tested these fixes in windows 11.

  • The traffic window and device windows were currently displaying smaller than their requested "minimum content sizes", and these minimum sizes were not enforced when resizing the window..
  • The traffic window and device windows were not always showing the top and left sides of the content in these windows.
  • The "titles" at the top of the traffic window and device windows were only displayed when you scrolled to the top of the list.
  • The details window didn't get a vertical scroll bar when needed.
  • The details window minimum size wasnt enforced.

This does not fix:

  • The traffic and device windows do not support horizontal scrolling.
    I think this might be due to an issue in the 'traffic_view' and 'device_view' related code, rather than the UI init code that was changed here. My reason for this thinking is that when I set the horizontal scrollbar to "always" be displayed for traffic and device windows, it shows the scroll bar as taking up the full space and so unable to be moved left or right. This is the scrollbar behaviour you get when it thinks the contents already fits into the scrollable window, and so doesn't need scrolling. So it seems like 'traffic_view' and 'device_view' are maybe misrepresenting the size of their rows.

@martinling
Copy link
Member

I've just tried this on Linux, and it doesn't really seem like an improvement to me. It just stops me from moving the pane dividers freely.

We don't actually want to enforce minimum sizes for the panes. The only reason I added those sizes was that they cause the panes to appear with reasonable initial proportions (e.g. ~75% traffic pane / ~25% device pane) at startup, rather than being split down the middle by default. They also cause the window to appear at a reasonable initial size.

The problems with the existing behaviour in my view are:

  • The list views do not always expand horizontally to make full use of the pane width.
  • When the pane is narrow, the pane does not prioritise showing the left edge of the view.

This change does stop the left edge of the traffic view from disappearing as the window size is reduced, but only does so by preventing the user from moving the pane dividers.

What we need is for the list views to always follow the size of the panes.

@x0rloser
Copy link
Contributor Author

In my initial commit the pane divders were able to be moved, but the minimum content sizes for the traffic and device windows were enforced. When the app first starts, the size of these windows were currently at their minimum sizes. This means that the dividers were not able to be moved until the window was enlarged.

Based on your feedback I have added a new commit which removes the explicit minimum content sizes. It seems that the scroll windows internally still calculate some "minimum content" sizes, however these are quite small so the user isn't inconvenienced by them and the divders can be chaned when the app first starts.

I also added in some calculations to set the initial size of the traffic window as a percentage of the horizontal and vertical panes. These values are not pixel perfect since the actual sizes of the app window, action bar, status bar etc are not available at init time, and there may be some system specific differences that makes the values slightly different between windows, mac, linux etc. However the values calculated should produce acceptable initial settings for the UI, which are better than we have currently. The user is able to then change them if needed at runtime, without any real limitations.

As before I have only tested this in Windows 11.
It would be good to test on mac on linux too.

I choose values for "traffic_width_percent", "traffic_height_percent", "app_width", "app_height" that I thought worked well. However you may wish to use some other values for these.

@martinling
Copy link
Member

Just to summarise some further discussion on this that @x0rloser and I had on Discord:

With the latest commit, I had two issues:

  1. The startup proportions are as desired if the application gets the window size it asks for, which will happen by default on many systems. However, when started in a tiling window manager, the application gets a window which is larger than requested, so the sizes specified for the panes don't produce the right proportions:
    image
  2. It's no longer possible to collapse any of the panes completely. They each have a small minimum size.

I suspect that what we need to do is connect some signal on the ApplicationWindow, perhaps show, that will be emitted once the size is known. Then we can set the proportions of the panes accordingly the first time the window is shown.

@martinling martinling marked this pull request as draft November 2, 2024 14:35
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.

2 participants