Skip to content

Commit

Permalink
[newsfeed] Suppress unsightly animation edge cases when there are 0 o…
Browse files Browse the repository at this point in the history
…r 1 active news items (#3336)

When the newsfeed module has an items list of size 1, every
`updateInterval` the animation runs to transition from the active story
to itself. This is unsightly. This PR suppresses that.

To reproduce: configure newsfeed with a single news source,
`ignoreOldItems` true, a short `updateInterval` (e.g. 3000), and a
carefully-chosen small `ignoreOlderThan` lining up with the current
contents of your news source.
  • Loading branch information
crazyscot authored Jan 14, 2024
1 parent b47600e commit dadc7ba
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 7 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ _This release is scheduled to be released on 2024-04-01._
### Fixed

- Skip changelog requirement when running tests for dependency updates (#3320)
- [newsfeed] Suppress unsightly animation cases when there are 0 or 1 active news items
- [newsfeed] Always compute the feed item URL using the same helper function

### Deleted

Expand Down
37 changes: 31 additions & 6 deletions modules/default/newsfeed/newsfeed.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,27 +118,33 @@ Module.register("newsfeed", {

//Override template data and return whats used for the current template
getTemplateData () {
if (this.activeItem >= this.newsItems.length) {
this.activeItem = 0;
}
this.activeItemCount = this.newsItems.length;
// this.config.showFullArticle is a run-time configuration, triggered by optional notifications
if (this.config.showFullArticle) {
this.activeItemHash = this.newsItems[this.activeItem]?.hash;
return {
url: this.getActiveItemURL()
};
}
if (this.error) {
this.activeItemHash = undefined;
return {
error: this.error
};
}
if (this.newsItems.length === 0) {
this.activeItemHash = undefined;
return {
empty: true
};
}
if (this.activeItem >= this.newsItems.length) {
this.activeItem = 0;
}

const item = this.newsItems[this.activeItem];
this.activeItemHash = item.hash;

const items = this.newsItems.map(function (item) {
item.publishDate = moment(new Date(item.pubdate)).fromNow();
return item;
Expand All @@ -150,7 +156,7 @@ Module.register("newsfeed", {
sourceTitle: item.sourceTitle,
publishDate: moment(new Date(item.pubdate)).fromNow(),
title: item.title,
url: this.getUrlPrefix(item) + item.url,
url: this.getActiveItemURL(),
description: item.description,
items: items
};
Expand Down Expand Up @@ -312,8 +318,27 @@ Module.register("newsfeed", {
if (this.timer) clearInterval(this.timer);

this.timer = setInterval(() => {
this.activeItem++;
this.updateDom(this.config.animationSpeed);

/*
* When animations are enabled, don't update the DOM unless we are actually changing what we are displaying.
* (Animating from a headline to itself is unsightly.)
* Cases:
*
* Number of items | Number of items | Display
* at last update | right now | Behaviour
* ----------------------------------------------------
* 0 | 0 | do not update
* 0 | >0 | update
* 1 | 0 or >1 | update
* 1 | 1 | update only if item details (hash value) changed
* >1 | any | update
*
* (N.B. We set activeItemCount and activeItemHash in getTemplateData().)
*/
if (this.newsItems.length !== this.activeItemCount || this.activeItemHash !== this.newsItems[0]?.hash) {
this.activeItem++; // this is OK if newsItems.Length==1; getTemplateData will wrap it around
this.updateDom(this.config.animationSpeed);
}

// Broadcast NewsFeed if needed
if (this.config.broadcastNewsFeeds) {
Expand Down
4 changes: 3 additions & 1 deletion modules/default/newsfeed/newsfeedfetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* MIT Licensed.
*/

const crypto = require("node:crypto");
const stream = require("node:stream");
const FeedMe = require("feedme");
const iconv = require("iconv-lite");
Expand Down Expand Up @@ -67,7 +68,8 @@ const NewsfeedFetcher = function (url, reloadInterval, encoding, logFeedWarnings
description: description,
pubdate: pubdate,
url: url,
useCorsProxy: useCorsProxy
useCorsProxy: useCorsProxy,
hash: crypto.createHash("sha256").update(`${pubdate} :: ${title} :: ${url}`).digest("hex")
});
} else if (logFeedWarnings) {
Log.warn("Can't parse feed item:");
Expand Down

0 comments on commit dadc7ba

Please sign in to comment.