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

EventListWidget: Event list widget for dashboard #4431

Closed
wants to merge 1 commit into from

Conversation

pfalcon
Copy link
Contributor

@pfalcon pfalcon commented Dec 30, 2024

Based on FollowUpWidget and offering similar functionality, just for securities' events.

Based on FollowUpWidget and offering similar functionality, just for
securities' events.
@pfalcon
Copy link
Contributor Author

pfalcon commented Dec 30, 2024

This patch is intended to be alternative to #2623 , to get the matter of event list widget going. Unlike that PR, which packs many commits into single PR, with some coding and implementation issues, this PR tries to produce as simple as possible event list widget, by reusing as much of existing code infra as possible.

@buchen
Copy link
Member

buchen commented Jan 7, 2025

Hi @pfalcon, thanks for trimming down the big pull request. I started picking up this request, but wasn't able to finish it last weekend. I want do change a couple things I noticed: there are three types of events, it is not clear which is which as the current display does not tell. And I think for events it could make sense to use full reporting periods to decide which to show (I have 400+ "past" which makes it hard to use at the moment). Next weekend I hope to complete that work.

@pfalcon
Copy link
Contributor Author

pfalcon commented Jan 8, 2025

@buchen: Thanks for looking into this!

And I think for events it could make sense to use full reporting periods to decide which to show (I have 400+ "past" which makes it hard to use at the moment).

As mentioned, it's based on the FollowUpWidget widget implementation, and thus offers "show past vs future dates" and "sort dates in ascending/descending order" settings. That allows to configure it to show next upcoming events (whichever number fits into the chosen vertical size), or recently passed events. IMHO, that's good enough for starters (on par with FollowUpWidget, and it's pretty useful, though of course we can think how it can be extended further either).

What's clearly missing is ability to filter by event type, but first there're not enough event types to start with! And adding those is completely separate rabbit hole, which I wanted to avoid going into right away.

@buchen
Copy link
Member

buchen commented Jan 9, 2025

@pfalcon do you have an opinion what to do with dividend events? They have two dates: the payment and the ex-dividend date. I am wondering if one wants to show both. Then we would have "synthetic" events which do not map to the underlying SecurityEvent class.

@pfalcon
Copy link
Contributor Author

pfalcon commented Jan 10, 2025

@buchen :

do you have an opinion what to do with dividend events? They have two dates: the payment and the ex-dividend date

I think these should be 2 separate (and independent) event types, then individual people can decide whether they want to track only one of them or both.

If there's an idea to track "complete" dividend cycle (kinda linking ex-div and payment date in some sequence), then IMHO that should be treated as a separate task/layer from just having a well-rounded inventory of simple single-date events.

@buchen
Copy link
Member

buchen commented Jan 10, 2025

If there's an idea to track "complete" dividend cycle (kinda linking ex-div and payment date in some sequence), then IMHO that should be treated as a separate task/layer from just having a well-rounded inventory of simple single-date events.

Agree.

The current functionality is limited. One can download dividend events from DivvyDiary. That events contain both - payment date and ex-date. I hope to add news sources this year.

buchen added a commit that referenced this pull request Jan 10, 2025
buchen added a commit that referenced this pull request Jan 10, 2025
@buchen
Copy link
Member

buchen commented Jan 10, 2025

I have now merged the change. A good start.

I think about two additions:

  • First: filter by events (all or only specific ones)
  • Second: instead of the "chart height", display 10 events. And then have a navigator (previous page, next page) if required. Right now, I find it confusing that there is no indicator that there are more events. I thought about adding scrolling, but this is super hard - a scrolled element within a scrolled page.

@pfalcon If you plan to contribute, let me know. I will not be able to work on this of the next couple days.

Bildschirmfoto 2025-01-10 um 18 35 58 Bildschirmfoto 2025-01-10 um 18 36 07

@buchen buchen closed this Jan 10, 2025
@pfalcon
Copy link
Contributor Author

pfalcon commented Jan 11, 2025

@buchen:

I have now merged the change. A good start.

Thanks! But I'm not sure that tying it into "reporting period" was really a good idea. Because what's for example a "1 year" reporting period as typical configured in dashboard? It's a period ending today and starting 365 days ago. In full accordance with that, the widget after you changes shows:

  • either events starting from 2024-01-12 (sorting direction: ascending)
  • or, events up to today, in descending order (sorting direction: descending)

First of the above is rather specialized and not very useful info, 2nd is more or less useful (assuming a user doesn't look at dashboard every day and want to know what they missed over the last few days). But the most useful info is upcoming events (ex-dividends, earnings releases all affect the share price and thus investment decisions), and that's exactly what "reporting period" widget can't show right away. If you look at the previous "event widget patch", then it apparently also was tied into reporting period, you can tell that by the fact that there were workarounds applied against the issue as described above: Fix current month and current year reporting period interval to include
dates in the future .

Apparently, the idea was to switch to "current month/year" reporting period specifically for the event list widget (away from "a period in the past" as normal for the entire dashboard). But that would still lead to problems on the period edges, like it wouldn't show events across a year transition like we just had.

But why all that? "Securities: Date reached" aka FollowUpWidget worked perfectly with it's "date in the future" vs "date in the past" setting, IMHO events shouldn't be tied into "reporting periods". (Reporting periods are to study performance across different timeframes, while events and not performance-related (at least per se)).

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