Add column with graphical price range widget #4274
Conversation
|
Screenshot: |
...ortfolio.ui/src/name/abuchen/portfolio/ui/util/viewers/ParameterizedColumnLabelProvider.java
Outdated
Show resolved
Hide resolved
name.abuchen.portfolio.ui/src/name/abuchen/portfolio/ui/views/columns/RangeWidgetColumn.java
Outdated
Show resolved
Hide resolved
|
|
||
| event.gc.setBackground(Colors.LIGHT_GRAY); | ||
| event.gc.setForeground(Colors.BLACK); | ||
| event.gc.fillRectangle(event.x, event.y + yOff, width, BAR_HEIGHT); |
There was a problem hiding this comment.
I propose not to paint the background here at all.
Two reasons: first, the selection marker will be gone. Second, depending on the platform there are alternating background of the table rows which will also be gone.
There was a problem hiding this comment.
I propose not to paint the background here at all.
Two reasons: first, the selection marker will be gone. Second, depending on the platform there are alternating background of the table rows which will also be gone.
Hmm, that's not what I tried to implement (per the docs), and not what I see here on Linux/GTK. First of all, owner-draw support for table cell consists of handling 3 events: MeasureItem, EraseItem, PaintItem. First 2 have default implementation on SWT level, and we rely on that. EraseItem is where things like backgrounds and selection are handled, and what I did is non-overridable implementation like:
+ // We're saying that we'll draw actual things ("foreground")
+ // ourselves. Things like background, selection, etc. will
+ // still be handled by SWT.
+ event.detail &= ~SWT.FOREGROUND;
That's all per https://www.eclipse.org/articles/Article-CustomDrawingTableAndTreeItems/customDraw.htm
Now, the actual widget consists of : a bar (rectangle), on which a notch (a line) is drawn. Both the bar and notch have their own colors, and that's what we draw. The bar is in particular drawn with top and bottom paddings from the actual cell size, exactly to make selection visible even in this column. Again, it all works as expected in GTK+ (well, I don't have an alternating table rows theme but would hope it works, as the selection does).
So, I wonder if that's one of those unfortunate platform-specific discrepancies in SWT. I'll try to research more as time permits.
There was a problem hiding this comment.
At least on macOS, it looks fine (and as expected) if we do not paint the background
There was a problem hiding this comment.
But now I get what you want to achieve - you want a grey bar and on top that the indicator with the black bar. Maybe we need to do some padding (on some platforms)?
name.abuchen.portfolio/src/name/abuchen/portfolio/math/AllTimeHigh.java
Outdated
Show resolved
Hide resolved
| final int BAR_HEIGHT = 16; | ||
|
|
||
| ReportingPeriod option = (ReportingPeriod) getOption(); | ||
| var range = valueProvider.apply(event.item.getData(), option); |
There was a problem hiding this comment.
I wonder if we need some caching here. The #paint method is called very often. On every selection, on every resize, on every right-click, etc.
If we need caching, I find it not so easy to say who to do the cache invalidation. There is the metricsCache in the security table. We could make that more generic and pass it the RangeWidgetColumn.
But maybe we do not caching?
There was a problem hiding this comment.
Well, there're definitely a lot of optimizations to do in PP in various places. For example, functional-like streaming interface to calculate stuff is nice and expressive (most of the time), but making 2 passes to calculate 2 values over the same items is clearly less efficient than ol' good loop calculating everything at once and possibly terminating early.
But optimizing prematurely also complicates things a lot, so it makes sense to have separate passes at that, hopefully driven by real quantified criteria. In that regard, I didn't really see noticeable slowdown with the widget enabled, even with pretty large (*) instrument lists. (I actually would think that SWT table widget uses virtual model approach and creates Items only for currently visible model rows, though I'm not really sure that's the case).
(*) Going for optimization efforts, we'd need some good reference to compare/measure things on, e.g. a list of S&P500 constituents, or something.
name.abuchen.portfolio.ui/src/name/abuchen/portfolio/ui/views/columns/RangeWidgetColumn.java
Outdated
Show resolved
Hide resolved
This label provider can now support drawing fully custom content: a subclass should just implement paint() method and call enableOwnerDraw() in constructor.
|
@buchen : I pushed new patch version which implements most of the suggestion you had (they marked as resolved). I also updated tooltip format a bit to show numeric relative percentages from high/low (so people could check for actual ATH/ATL reliably). What's left is the matter of painting body of the horizontal scale bar. Can you please make a screenshot of how it looks for you on MacOS to understand better why you think it needs changing? |
Period over which range is calculated is configurable, similar to "Distance from ATH" column.
This PR builds on ParameterizedColumnLabelProvider introduced in #4225, add to it owner-draw capabilities, as discussed in #4226, and implements on top of it column which represent security price range within given (configurable) time interval in a graphical form, where full range is represented by a horizontal bar, and a notch on it shows current relative position. That's how almost every stock analysis site shows price ranges, and what IMHO was sorely missed in PP. This new column type is expected to supersede "Distance from ATH" column, issues with which are: