-
Notifications
You must be signed in to change notification settings - Fork 334
network graph - show/hide panels based on window width/height #540
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
network graph - show/hide panels based on window width/height #540
Conversation
20220131135552215.mp4 |
5e4385e to
a1bd27f
Compare
jarolrod
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From a UX perspective, why would we ever want to hide access to the durations slider and the current in/out values? Perhaps we could make more efficient use of the space in displaying the slider and in/out values as was attempted in #90.
|
Part of what I have added to my thought process is how this will translate to the mobile U/I (android/etc) - I have implemented a hybrid version of both of these approaches - but haven't posted a PR to it. Some of my conclusions to these approaches (and the gui repo in general) - is that it may be beneficial to have a hybrid of both implementations - it would lend itself to a mobile portrait/landscape design idiom. I may be wrong - but there doesn't seem to be enough reviewing members/contributors that have experience in the nuances of cross platform design including mobile U/I support. I don't see the Android developers reviewing this stuff to ensure up/down stream compatibility - that seems weird/negligent to me, having done mobile UI design for web, macOS/(OSX) and iOS and (Apache/Cordova) work. |
|
How about moving the current values on top of the graph (top-left?) instead? |
|
@luke-jr - I agree - thanks for your suggestion - I will revisit this PR shortly |
a1bd27f to
6169d95
Compare
|
@luke-jr - how about a simple overlay? note: this approach removed 39 lines of code. |
|
That looks kind of hard to read IMO. Maybe just make it the default widget background behind the text? (QFrame for borders?) |
0b7d6ac to
3853930
Compare
|
@luke-jr - This is pretty close to the original colors... |
|
The correct colours to match "original" are going to vary based on the user's colour theme. But maybe as long as we make sure the text and background have good contrast, the theme doesn't need to apply within the overlay since the graph itself is unthemed. I do think the current screenshots are still a bit hard to read. Maybe bolding the text will do the trick? |
|
I will look into toggling the graph (light/dark) themes based on system preferences - it may be a good "feature". |
The debugwindow - network graph hides ui elements btnClearTrafficGraph, lblGraphRange, sldGraphRange when at minimumHeight, it also hides the groupBox at minimumWidth - this maximizes the dimensions of the traffic graph in these small dimensions.
3853930 to
bb78ade
Compare
|
I have adjusted the colors based on the global template and an accessibility audit. If we are going to be this nuanced - please actually compile/run the PR changes. [deleted] |
traffic_graph_a11y_audit_2.mov |
bb78ade to
ea33a9b
Compare
ea33a9b to
1afde28
Compare
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process. ConflictsNo conflicts as of last run. |
|
Closing this due to lack of activity. Feel free to reopen. |







The debugwindow - network graph hides ui elements
btnClearTrafficGraph, lblGraphRange, sldGraphRange
when at minimumHeight, it also hides the groupBox
at minimumWidth - this maximizes the dimensions of
the traffic graph in these small dimensions.