Tracking tab optimizations + new dynamic legend widget #179

keithel-qt · 2021-10-25T20:13:35Z

This optimizes series creation in the Tracking Signals tab.

Series are created once in QML, and filled whenever there are changes.

This reimplements the legend in the Tracking Signals tab

Two column, scrolling.
Can reposition it by dragging the titlebar
Can shutter it closed by clicking the titlebar

silverjam · 2021-10-25T22:08:49Z

The legend doesn't seem to be displaying on my Linux machine:

silverjam · 2021-10-25T23:10:28Z

I'm seeing it on windows, not sure why it's not showing up on Linux, let me run it by the apps team and see what they think. I think in general we can make the font size smaller to make the legend less obtrusive, I know there's a lot of signals but it really covers a lot of display area on the chart.

keithel-qt · 2021-10-25T23:16:00Z

I'm seeing it on windows, not sure why it's not showing up on Linux, let me run it by the apps team and see what they think. I think in general we can make the font size smaller to make the legend less obtrusive, I know there's a lot of signals but it really covers a lot of display area on the chart.

We could make it single column scrollable - that's another thing we can adjust.

keithel-qt · 2021-10-25T23:22:54Z

The legend doesn't seem to be displaying on my Linux machine:

I was initially having trouble with it showing up on Windows too. The way I calculate the column widths is a little complicated, since I base it off of the actual implicitWidths of the cells in the gridview (which is the colored rect plus the text). Is there some fixed width of cell I can go with that would simplify that calculation?
I guess I can sample and figure out what the widest cell implicitWidth is, and if a cell would be wider than that, elide the text.

keithel-qt · 2021-10-26T00:30:07Z

The legend doesn't seem to be displaying on my Linux machine:

!! I apparently didn't push my last commit. I already had a fix for a legend that didn't show.
3e8e607

This should show properly (though I think it's overly complex in how it determines how wide the legend should be).

silverjam

From comments by others on slack (I believe this was the consensus, correct me if you heard otherwise):

Leave in the ability to shade/hide the legend
Display by default (already done)
Disable ability to move the legend
Scollable if it overflows the max of 2 columns

john-michaelburke · 2021-10-26T19:14:14Z

resources/TrackingTabComponents/TrackingSignalsTab.qml

+                    }
                }
            }



The checkboxes seem to have disappeared for me

john-michaelburke · 2021-10-26T19:25:54Z

If you merge in tip of tree you may see performance boost. I believe there was a regression introduced that was fixed in:
76467b8

This is the first iteration of optimizing performance of the TrackingSignalsTab. Series' are created only once in QML, to associate them with the chart. Thereafter, the series' persist. Points are replaced and properties are set with each fill_all_series call - which occurs regularly as called by a timer from QML. The update mechanism remains unchanged. Legend seems broken with the changes I have put in here - despite my having attempted to accomodate it.

* This fixes my prior changes so the Tracking Signals tab legend actually shows (make an all_series changed notify signal, and emit it when the series changes, so the legend knows when to update. * This reimplements the legend so it looks nicer, based on guidelines from Jason. Maximum two column and scrolling when things don't fit. * Legend can be moved around the chart (drag title bar) * Legend can be hidden to show chart more clearly (click title bar) Rebase: Chart is not filling properly...

* Fix formatting issues * Clarify that IndexError is expected in fill_all_series

* This adds a notify property to all of the properties in the TrackingSignalsPoints object that did not have it. The lack of the notify property was preventing QML from seeing the changes in those properties, and thus the checkboxes were not being populated in the UI. * Instead of putting the chart and checkboxes in a Rectangle with anchoring, instead use a ColumnLayout. No need for the container to be visible, and it's better if it manages the height of the children.

* Cell width and height is now not dynamically calculated per cell, but based off reference cell contents, reducing complexity. * Legend now resizes as small as it can to the number of series elements. * Still keeps maximum of 2 columns, scrollable beyond that.

* This adds some of the SwiftOrange color styling to the TrackingSignals tab. * Remove redundant anchoring, use ColumnLayout in legend shade bar and legend grid

keithel-qt · 2021-10-28T01:35:24Z

Ok, I think this is in a state that it can be merged.

Reduce the font size we use for the Tracking Signals legend per Jason Mobarak's request.

john-michaelburke · 2021-10-28T17:46:30Z

Everything visually looks great to me! I have two concerns.

~~I think this PR may be introducing a bug~~ EDIT: I am seeing an unwanted bug and looks like it was there before this PR as well where unchecking a checkbox seems to not clear out what is shown but rather stop updating with new values. The result are these trailing lines which stop in time where you unchecked the box. I think additionally to the not updating we may need to keep track of the visibility parameter for all series to do this.
Not so much a concern but a nit. In removing the gray around the plot there is now a large chunk of whitespace between the x axis label and the checkboxes. Is there any way we could reduce this a bit? Maybe 1/3 give or take, of the space currently seen?

john-michaelburke · 2021-10-28T17:58:34Z

trailing-series.mp4

silverjam · 2021-10-28T19:15:51Z

Everything visually looks great to me! I have two concerns.

~~I think this PR may be introducing a bug~~ EDIT: I am seeing an unwanted bug and looks like it was there before this PR as well where unchecking a checkbox seems to not clear out what is shown but rather stop updating with new values. The result are these trailing lines which stop in time where you unchecked the box. I think additionally to the not updating we may need to keep track of the visibility parameter for all series to do this.

If this was a pre-existing bug, let's leave it for another PR... but what I'm seeing is a mix of behavior, some of the lines are completely hidden when the checkbox is toggled off and some of the lines turn in to "trailing lines" (almost as if there's two different series drawing the same visual line):

Not so much a concern but a nit. In removing the gray around the plot there is now a large chunk of whitespace between the x axis label and the checkboxes. Is there any way we could reduce this a bit? Maybe 1/3 give or take, of the space currently seen?

Agree, we should make the following changes:

Reduce the font size of the tracking chart title, and make it a "regular" font face, not bold
Grow the size of the chart in the X and Y direction so that it fills more space

silverjam · 2021-10-28T23:16:17Z

Going to merge this, we'll need to create a new PR to follow-up on these requests: #179 (comment)

silverjam · 2021-10-29T00:40:12Z

Going to merge this, we'll need to create a new PR to follow-up on these requests: #179 (comment)

Follow-up task: #196

keithel-qt requested review from john-michaelburke and silverjam October 25, 2021 20:13

keithel-qt force-pushed the keithel-qt/trackingtab-optimizations branch from ec6b26c to 85e4ccc Compare October 26, 2021 16:38

silverjam reviewed Oct 26, 2021

View reviewed changes

john-michaelburke reviewed Oct 26, 2021

View reviewed changes

resources/TrackingTabComponents/TrackingSignalsTab.qml

}

}

}

Copy link

Collaborator

john-michaelburke Oct 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The checkboxes seem to have disappeared for me

keithel-qt added 8 commits October 27, 2021 20:25

This fixes legend showing in TrackingSignalsTab

3da5cc1

Fix formatting issues, clarify IndexError exception

90059fb

* Fix formatting issues * Clarify that IndexError is expected in fill_all_series

Make the legend borderline nicer

75a9e55

Improve color, layout formatting

ad18e9f

* This adds some of the SwiftOrange color styling to the TrackingSignals tab. * Remove redundant anchoring, use ColumnLayout in legend shade bar and legend grid

keithel-qt force-pushed the keithel-qt/trackingtab-optimizations branch from 85e4ccc to ad18e9f Compare October 28, 2021 01:26

Formatting fixes from make check-all

9cdc00a

Reduce legend font size

9d7738a

Reduce the font size we use for the Tracking Signals legend per Jason Mobarak's request.

silverjam changed the title ~~Keithel qt/trackingtab optimizations~~ Tracking tab optimizations + new dynamic legend widget Oct 28, 2021

silverjam merged commit d4c710b into main Oct 28, 2021

silverjam deleted the keithel-qt/trackingtab-optimizations branch October 28, 2021 23:17

silverjam mentioned this pull request Oct 28, 2021

Follow-up on tracking view improvements #196

Closed

silverjam restored the keithel-qt/trackingtab-optimizations branch October 29, 2021 00:40

silverjam deleted the keithel-qt/trackingtab-optimizations branch October 29, 2021 00:40

Tracking tab optimizations + new dynamic legend widget #179

Tracking tab optimizations + new dynamic legend widget #179

Uh oh!

Conversation

keithel-qt commented Oct 25, 2021

Uh oh!

silverjam commented Oct 25, 2021

Uh oh!

silverjam commented Oct 25, 2021

Uh oh!

keithel-qt commented Oct 25, 2021

Uh oh!

keithel-qt commented Oct 25, 2021

Uh oh!

keithel-qt commented Oct 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

silverjam left a comment

Choose a reason for hiding this comment

Uh oh!

john-michaelburke Oct 26, 2021

Choose a reason for hiding this comment

Uh oh!

john-michaelburke commented Oct 26, 2021

Uh oh!

keithel-qt commented Oct 28, 2021

Uh oh!

john-michaelburke commented Oct 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

john-michaelburke commented Oct 28, 2021

Uh oh!

silverjam commented Oct 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

silverjam commented Oct 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

silverjam commented Oct 29, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

keithel-qt commented Oct 26, 2021 •

edited

Loading

john-michaelburke commented Oct 28, 2021 •

edited

Loading

silverjam commented Oct 28, 2021 •

edited

Loading

silverjam commented Oct 28, 2021 •

edited

Loading