Skip to content

Add per widget update intervals with possilbe configuration in deck files#31

Merged
muesli merged 2 commits intomuesli:masterfrom
zMoooooritz:update-intervals
Jun 7, 2021
Merged

Add per widget update intervals with possilbe configuration in deck files#31
muesli merged 2 commits intomuesli:masterfrom
zMoooooritz:update-intervals

Conversation

@zMoooooritz
Copy link
Copy Markdown
Contributor

Just a short heads up!

This is more or less the same that I already had impelmented in #22

I decided to place the interval right inside the WidgetConfig since this is a setting that can be applied to every widget.
I would have loved to model the interval as int but this would mean that widgets without a specified interval get the "default value" 0 which is not desireable.
Furthermore I'm not really happy with the setInterval call in every switch-case but I don't know how I could avoid this.

@muesli muesli force-pushed the update-intervals branch from fd30215 to 3493919 Compare June 7, 2021 00:02
@muesli
Copy link
Copy Markdown
Owner

muesli commented Jun 7, 2021

This is more or less the same that I already had impelmented in #22

Awesome, thanks for porting that over!

I decided to place the interval right inside the WidgetConfig since this is a setting that can be applied to every widget.

Sounds good.

I would have loved to model the interval as int but this would mean that widgets without a specified interval get the "default value" 0 which is not desireable.

I just simplified that logic a bit, so we can use a uint here. The default value zero now means the widget will only see an initial paint, with no further repaints. I've also managed to remove the need for a separate initialized bool, and implemented a specific RequiresUpdate method for the RecentWindowWidget.

Furthermore I'm not really happy with the setInterval call in every switch-case but I don't know how I could avoid this.

  1. Instead of directly returning a newly generated Widget, we could store it a var first and then have one generic setInterval call outside the switch statement. This works because all widgets we create match the Widget interface, so they can be kept in the same type of variable.
  2. Instead of manually setting a default interval, we could implement widget specific RequiresUpdate functions, that for cases where w.Interval is zero (the default) apply their own default interval. This keeps the logic inside the widget itself and widget.go becomes a bit less coupled to the individual widgets.

@muesli muesli added the enhancement New feature or request label Jun 7, 2021
@muesli muesli merged commit 30aba0a into muesli:master Jun 7, 2021
@zMoooooritz
Copy link
Copy Markdown
Contributor Author

I just simplified that logic a bit, so we can use a uint here. The default value zero now means the widget will only see an initial paint, with no further repaints.

Not a huge of using zero as "no repaints required" but for the sake of cleaner code it is ok.

  1. Instead of directly returning a newly generated Widget, we could store it a var first and then have one generic setInterval call outside the switch statement. This works because all widgets we create match the Widget interface, so they can be kept in the same type of variable.
  2. Instead of manually setting a default interval, we could implement widget specific RequiresUpdate functions, that for cases where w.Interval is zero (the default) apply their own default interval. This keeps the logic inside the widget itself and widget.go becomes a bit less coupled to the individual widgets.

Like the way you moved the attribute unpacking to the widgets. This looks so much cleaner now!

@zMoooooritz zMoooooritz deleted the update-intervals branch June 7, 2021 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants