Add trending tags

[DavidEdwards]

Related to #2109.

This adds the ability to add a trending tab to the bottom navigation bar.

Includes a new custom view for graphing. This could perhaps be made nicer with an external library.

Only hashtags are supported currently, there are two other trending APIs that could eventually be added.

• Fix failed checks
• Add error handling (see airplane mode)
• Black theme, graph background should be black

[nikclayton]

I'll send separate feedback on the code, but some high level stuff.

Appearance

It's probably easier if I show an alternative layout, and describe what I think it improves:

(based on https://dribbble.com/shots/2513500-Stats-card/attachments/9339291?mode=media)

• It can scale to the width of the screen, all the space is used to display useful data.

• The date range of the chart is indicated, without needing markers under the X axis

• The high/low numbers are marked (they could also be shown as smaller text, perhaps underneath the larger usage and accounts numbers?). I think they're optional, but probably nice to have.

• This also gives a sense of the scale of the charts, without needing to explicitly label the Y axis.

• Very little text would be repeated across charts (just the date range and labels). Subjectively, this feels less busy than the repeated "X people are talking" text.

(Colours should match the Tusky colours, of course).

More examples for inspiration; https://chartio.com/blog/new-chart-type-sparklines/, https://dribbble.com/shots/2716084-Study-of-sparkline-like-graphics/attachments/9446250?mode=media, https://www.thesmallman.com/excel-sparklines

Also: I know the API only hands back 7 days worth of data, which might not be enough to make a full-width view work (the mockup above has 30 data points).

If that's not enough data, how about a two column layout using GridLayoutManager, where each entry in the grid looks like this:

?

General

Since there are other trending options available (statuses, links) it's probably best if the name is more specific in both the UI and the code, so that when those others are added it's not an issue.

Speaking of that, have you given any thought to how those others would be shown in the UI / how it will scale to show them / how the user will switch between the ones they want?

Optional: The list of trending hashtags in the results includes ones that I am actively filtering. That's consistent with what the Mastodon website does, but maybe there's an opportunity to do better than the website, and filter them from this view too?

If the user doesn't add the Trending tab, can it be added to the main navigation drawer as an entry there? For example, if the user disables the top navigation then the Search menu (which is normally there) moves to the Navigation drawer. This could behave the same way, so it's accessible to users that want it semi-frequently, but don't want to give up an entire tab for it.

[nikclayton]

I hope that's helpful feedback.

[DavidEdwards]

Thank you for the very thorough feedback. I'll work through it over the next several days.

You are correct, the 7 days of data is the problem. I like the idea of using a grid. That could also scale nicely for tablets.

The explicitly provided date range makes sense too.

Is it worth showing both the account and usage graphs? I don't know whether the data is interesting enough to show both. The peaks should be mostly identical. The accounts graph is just more likely to be flatter. I would be interested to hear your perspective.

I completely agree with the repeated text argument. The new design certainly feels like it provides more information to the user.

Regarding the code, a lot of the structures are copy-pasted-adapted from existing code. Hence the java file(s), findByViewId, etc. I feel like I am getting more comfortable with the codebase, so cleaning that up shouldn't be a hassle.

[nikclayton]

Is it worth showing both the account and usage graphs? I don't know whether the data is interesting enough to show both. The peaks should be mostly identical. The accounts graph is just more likely to be flatter. I would be interested to hear your perspective.

Maybe... the info's there, so perhaps it's useful? I don't a very strong opinion, and I can see how it could also make the charts harder to read. I suspect it's a "try it and see" situation.

[DavidEdwards]

@nikclayton I have replied to your comments, and here is a visual of the current cells.

[DavidEdwards]

Since there are other trending options available (statuses, links) it's probably best if the name is more specific in both the UI and the code, so that when those others are added it's not an issue.

I will need to look at this more closely. In the UI, it could be labelled Trending Tags perhaps (though, something more generic would be nice... Trending Graphs?). In the code, could you give an example? Would TrendingViewData be confusing currently?

Speaking of that, have you given any thought to how those others would be shown in the UI / how it will scale to show them / how the user will switch between the ones they want?

Not yet, but I think that will be my next task. Trending links has a similar structure, I will likely look there.

Optional: The list of trending hashtags in the results includes ones that I am actively filtering. That's consistent with what the Mastodon website does, but maybe there's an opportunity to do better than the website, and filter them from this view too?

I think that makes sense. I will need to investigate how the filtering happens.

If the user doesn't add the Trending tab, can it be added to the main navigation drawer as an entry there? For example, if the user disables the top navigation then the Search menu (which is normally there) moves to the Navigation drawer. This could behave the same way, so it's accessible to users that want it semi-frequently, but don't want to give up an entire tab for it.

I see, I think that makes sense.

[nikclayton]

That is looking pretty sweet. I haven't looked at the code yet or your other replies, but some quick feedback on the new UI:

1. My bad, there's probably no need to repeat the dates in each chart's view. Could they be added to a single view above the list of charts?

I thought about in the title bar, but there may not be enough room, especially in different locales. So above the list probably makes sense. I'm not sure if it should scroll (so it's the first item in the list), or if it should be fixed (so should be a separate view).

2. The title should probably be "Trending hashtags" ("hashtags" for consistency with how else the term is used in the app)

[Although -- I could imagine that the future presentation of this stuff is in a TabLayout with different tabs for hashtags, statuses, and links. Maybe that's worth doing now, even if there's only one tab?]

3. Can the Y-axis of all the charts be scaled to the same maximum (i.e., whatever the largest values is). At the moment, glancing at the charts it looks as though they're all equally popular, which is not the case.

4. Can a tick mark be added on the X-axis to indicate each day?

5. In the examples in the screenshot they all have non-zero values. But the chart lines all (except for "#exxon" for some reason) go back down to zero. Bug?

6. Not sure the FAB makes sense here. Maybe a small "post" icon button on each hashtag listitem, that would create a new post populated with that tag?

Although since clicking the view takes you to the search page for that hashtag, maybe that would be better implemented as a FAB on the hashtag activity, and avoid further content on each listitem?

[DavidEdwards]

there's probably no need to repeat the dates in each chart's view.

Sounds good. I added to the start of the list. Since otherwise it would take up too much screen real estate in landscape.

The title should probably be "Trending hashtags"

I will use this for now. The TabLayout would make sense in the future.

Can the Y-axis of all the charts be scaled to the same maximum (i.e., whatever the largest values is). At the moment, glancing at the charts it looks as though they're all equally popular, which is not the case.

Absolutely. There is already a proportionalTrending boolean for the GraphView. I will activate that.

Can a tick mark be added on the X-axis to indicate each day?

Sure!

In the examples in the screenshot they all have non-zero values. But the chart lines all (except for "#exxon" for some reason) go back down to zero. Bug?

I will double check this.

Not sure the FAB makes sense here. Maybe a small "post" icon button on each hashtag listitem, that would create a new post populated with that tag? Although since clicking the view takes you to the search page for that hashtag, maybe that would be better implemented as a FAB on the hashtag activity, and avoid further content on each listitem?

When you click on the tag, it takes you to the list where you can create a post with the FAB. Perhaps that is enough. I will disable it for now.

[DavidEdwards]

In the examples in the screenshot they all have non-zero values. But the chart lines all (except for "#exxon" for some reason) go back down to zero. Bug?

I will double check this.

Looks like I found a bug.

However, there are commonly troughs on the latest day, especially early in the day when the value isn't as populated yet as the previous day.

[connyduck]

Thx you very much!

Cut off hashtags are a no-go here. It is the most relevant info. Also why is a gap between # and name, we don't have that anywhere else.

Eventually we should add a library that can draw really nice graphs, for now this is ok I guess.

[DavidEdwards]

@connyduck @nikclayton I could do with final clarification on the sidebar for trending and whether the FAB should be shown on trending screens. To avoid making unnecessary changes.

[nikclayton]

Ultimately, @connyduck has final approval. My arguments for the suggested changes:

Nav drawer

It should have an entry on the navigation drawer because if it doesn't, the only way the user is going to discover this feature is if they explicitly add a tab for it. That's buried in the UI, and is very non-discoverable.

Out of the box, I think useful features like this should be discoverable so that users have a good first impression with the app and are not left confused.

For an example of how this doesn't work look at how there needs to be a FAQ entry for "How do I see the federated timeline?", and my public timeline of posts is filled with me replying to people sending them a link to the FAQ.

We already have preferences to e.g., hide qualitive stats on posts. If there's some concern that users might never want to see trends an additional pref to show trends could be added, default to "On".

No FAB

The FAB is for the most common or important action on the screen (https://m3.material.io/components/floating-action-button/overview#13a9b2a5-13f2-4617-a08b-e66576df55b0)

When viewing trends, I don't think "Create a new post" is the most common or important action. I think being able to see the trends is.

The FAB blocks the bottom-right trend, and in some cases there's no way to remove that (e.g., the trends take up just enough space to fill the screen without scrolling).

If the user sees a hashtag and wants to create a post using it they can click through to the hashtag (which should do a search), and then there should be a FAB on that page (aside: at the moment there isn't).

[DavidEdwards]

It looks like I pushed at the same time as your commented, the current system removes the FAB and does not add to the nav drawer. I left the drawer code as an option for future tickets though.

[nikclayton]

I think I may have been wrong to suggest that the default grid should have two columns. I see

#songsormoviesabout
beinglost

#meteorologicalmelod
ies

and

#holocaustmemoriald
ay

which isn't a great look.

I've just pushed an alternative to my branch, single column in portrait, 3 column when wider, looks like this:

and this:

WDYT?

[DavidEdwards]

which isn't a great look.

That is true, but there will be a point when that will happen anyway. Though I suppose that if we can ensure the 99% cases don't wrap. That is good.

Incidentally, why the removal of the bold? I think that is good to highlight the important information.

I will take a look over the changes when I get a chance, but one thing from the screenshots stands out:

The line chart looks to be the complete cell height. I can't guarantee that isn't my fault thought.

[nikclayton]

which isn't a great look.

That is true, but there will be a point when that will happen anyway. Though I suppose that if we can ensure the 99% cases don't wrap. That is good.

Yeah. I tried ellipsize and marquee attributes, to see if the tag could scroll if overlong, but couldn't get them to work (possibly user error on my part).

Incidentally, why the removal of the bold? I think that is good to highlight the important information.

1. Made the text a bit narrower
2. Consistent with text styling in other Android lists
3. I felt that the difference in text size was sufficient to delineate the info

I will take a look over the changes when I get a chance, but one thing from the screenshots stands out:

The line chart looks to be the complete cell height. I can't guarantee that isn't my fault thought.

Not your fault, I experimented with that.

1. It saves some vertical space
2. It clearly ties the text to the chart, which is necessary without dividers between them (otherwise you can't at a glance tell if the text is referring to the chart immediately below or immediately above it.

"Why remove the dividers?" you might be thinking.

I'm a big fan of Tufte's "The Visual Display of Quantitative Information" (https://faculty.salisbury.edu/~jtanderson/teaching/cosc311/fa21/files/tufte.pdf), in which he writes (talking about printed material, but it's just as relevant to the screen):

A large share of ink on a graphic should present data-information, the ink changing as the data change. Data-ink is the non-erasable core of a graphic, the non-redundant ink arranged in response to variation in the represented. [p93, but the whole section from p84 onwards is really worth a read, you'll never look at a badly put together presentation the same way again :-)]

s/ink/pixels/, and you get the data/pixels ratio, the number of pixels that convey data / the total number of pixels in the graphic.

So with these UI experiments I'm trying to see what's the most useful information we can comfortably convey with the fewest pixels. By, e.g., removing the dividers.

The legend might still need some work if data points go under it -- a transparent white background to provide some visual separation maybe -- but I haven't tried that yet.

[DavidEdwards]

It clearly ties the text to the chart, which is necessary without dividers between them

I meant specifically this chart cell. The line will be underneath the text in some cases. That really don't feel correct.

fun setup(start: Date, end: Date) {
I am usually a fan of multi-line parameter function definitions. It makes it easier to see changes in diffs. Though, with two small variables this is pretty debatable.

private val binding: ItemTrendingCellBinding
I don't think that it is necessary to remove the trailing comma. I explicitly add those so that new changes don't cause an additional line in a diff.

KMGTPE
This confuses me. GTPE are very tech terms, and don't really match accounts / uses. For example 100G could appear under accounts. Giga accounts?

TrendingPagingAdapter — absolutely, it used to use the Paging3 system. Paging really makes no sense for this one.

Filters — This is great, I wanted to add this, so you saved me a lot of fiddling.

Line Width — I didn't realise that sp is used for these dimensions. I thought that sp was only for fonts. TIL.

The graphing X Axis makes sense instead of the dividers.

[nikclayton]

It clearly ties the text to the chart, which is necessary without dividers between them

I meant specifically this chart cell. The line will be underneath the text in some cases. That really don't feel correct.

Yes -- that's why I think the text views might need a translucent background, to cover that case and ensure they're legible. Or maybe a white stroke around the letters?

It might be worth constructing some fake pathological data (say, 5 days of the value being 100, and the last two days being 50), and see the UI looks like in that case. If it's unreadable even with a translucent background when pushing the labels back above the chart is probably the simplest solution.

fun setup(start: Date, end: Date) { I am usually a fan of multi-line parameter function definitions. It makes it easier to see changes in diffs. Though, with two small variables this is pretty debatable.

I agree. And if it's going to grow to need many other parameters then the churn in the function body probably dominates the churn caused by changing the arg list.

private val binding: ItemTrendingCellBinding I don't think that it is necessary to remove the trailing comma. I explicitly add those so that new changes don't cause an additional line in a diff.

Normally I would agree with you here as well. Newer versions of ktlint disagree however, and I've learned that if there's a decent formatter / linter then it's generally much simpler to just go with that.

We're not using those newer versions of ktlint via gradlew ktlintCheck yet, but it'll be an issue when we do. And Android Studio ktlint plugins are using the new versions of ktlint now, so I figured it was good to be ahead of the curve, ready for when we do update the ktlint version (#2914).

KMGTPE This confuses me. GTPE are very tech terms, and don't really match accounts / uses. For example 100G could appear under accounts. Giga accounts?

Think of it as extreme futureproofing :-) Realistically I can't imagine we need more than "K" at the moment, but I didn't think it hurt to include.

Line Width — I didn't realise that sp is used for these dimensions. I thought that sp was only for fonts. TIL.

You can use sp for anything (I think). The key property is that it will scale if the user changes the font size on their device.

I suspect that if the user's changing their font size then anything else that's thin lines should probably scale as well.

But dp would also work, and would scale if the user uses Android settings to scale their display.

[DavidEdwards]

It clearly ties the text to the chart, which is necessary without dividers between them

I meant specifically this chart cell. The line will be underneath the text in some cases. That really don't feel correct.

Yes -- that's why I think the text views might need a translucent background, to cover that case and ensure they're legible. Or maybe a white stroke around the letters?

It might be worth constructing some fake pathological data (say, 5 days of the value being 100, and the last two days being 50), and see the UI looks like in that case. If it's unreadable even with a translucent background when pushing the labels back above the chart is probably the simplest solution.

I see, that might work.

private val binding: ItemTrendingCellBinding I don't think that it is necessary to remove the trailing comma. I explicitly add those so that new changes don't cause an additional line in a diff.

Normally I would agree with you here as well. Newer versions of ktlint disagree however, and I've learned that if there's a decent formatter / linter then it's generally much simpler to just go with that.

We're not using those newer versions of ktlint via gradlew ktlintCheck yet, but it'll be an issue when we do. And Android Studio ktlint plugins are using the new versions of ktlint now, so I figured it was good to be ahead of the curve, ready for when we do update the ktlint version (#2914).

Oh, that is news to me. I had just persuaded my colleagues to start adding trailing commas too. 😞

KMGTPE This confuses me. GTPE are very tech terms, and don't really match accounts / uses. For example 100G could appear under accounts. Giga accounts?

Think of it as extreme futureproofing :-) Realistically I can't imagine we need more than "K" at the moment, but I didn't think it hurt to include.

No I don't mean it is bad. The concept is fine! I don't think the technical terms make sense though. K is often used for thousands, M for millions, B for billions, for example. In terms of generic quantities, I think using G would confuse a lot of people, since we aren't talking about bytes, for example. Am I wrong in this?

Line Width — I didn't realise that sp is used for these dimensions. I thought that sp was only for fonts. TIL.

You can use sp for anything (I think). The key property is that it will scale if the user changes the font size on their device.

I suspect that if the user's changing their font size then anything else that's thin lines should probably scale as well.

But dp would also work, and would scale if the user uses Android settings to scale their display.

Ok, fair enough.

[nikclayton]

It clearly ties the text to the chart, which is necessary without dividers between them

I meant specifically this chart cell. The line will be underneath the text in some cases. That really don't feel correct.

Yes -- that's why I think the text views might need a translucent background, to cover that case and ensure they're legible. Or maybe a white stroke around the letters?
It might be worth constructing some fake pathological data (say, 5 days of the value being 100, and the last two days being 50), and see the UI looks like in that case. If it's unreadable even with a translucent background when pushing the labels back above the chart is probably the simplest solution.

I see, that might work.

I just pushed that to my branch, in these screenshots it's the third item, #photomonday

I set the tick marks to 5% of the height as well to see what that would do.

That's about as far as I'm going to take this particular experiment.

private val binding: ItemTrendingCellBinding I don't think that it is necessary to remove the trailing comma. I explicitly add those so that new changes don't cause an additional line in a diff.

Normally I would agree with you here as well. Newer versions of ktlint disagree however, and I've learned that if there's a decent formatter / linter then it's generally much simpler to just go with that.
We're not using those newer versions of ktlint via gradlew ktlintCheck yet, but it'll be an issue when we do. And Android Studio ktlint plugins are using the new versions of ktlint now, so I figured it was good to be ahead of the curve, ready for when we do update the ktlint version (#2914).

Oh, that is news to me. I had just persuaded my colleagues to start adding trailing commas too. 😞

KMGTPE This confuses me. GTPE are very tech terms, and don't really match accounts / uses. For example 100G could appear under accounts. Giga accounts?

Think of it as extreme futureproofing :-) Realistically I can't imagine we need more than "K" at the moment, but I didn't think it hurt to include.

No I don't mean it is bad. The concept is fine! I don't think the technical terms make sense though. K is often used for thousands, M for millions, B for billions, for example. In terms of generic quantities, I think using G would confuse a lot of people, since we aren't talking about bytes, for example. Am I wrong in this?

I'm not sure if you are or not.

I went looking to see if there are locale-aware versions of these suffixes, and I couldn't find any (I spot checked the ICU data at https://github.com/unicode-org/icu/tree/main/icu4c/source/data/unit). It looks like M, G, etc are common in abbreviations like "MW" and "GHz", etc. For example, https://github.com/unicode-org/icu/blob/main/icu4c/source/data/unit/he.txt is Hebrew (I think) but still uses "G" in "GHz".

On API 30 or above we could use https://developer.android.com/reference/kotlin/android/icu/number/NumberFormatter

[nikclayton]

OK, one more, and then I really am done. The spiky lines bugged me, so I made them bezier's instead for a bit of a curve. WDYT?

[nikclayton]

OK, one last set, and then I really, really am done :-)

• Sort the tags by popularity, most uses first
• Make it clear in the legend that it's totals, not the current value
• Hide FAB
• Show the current values at the end of the chart

I'm not sure about that last one -- sometimes it works quite well (e.g., in the first row), other times it looks a bit clunky, like the second one

[nikclayton]

Irrespective of the UI stuff, there's a bug where the menu item is not added to the drawer if trending isn't present as a tab. I'm just taking a look at that now.

Edit:

MainActivity.onCreate calls setupDrawer with addTrendingButton = false

Then it observes MainTabsChangedEvent, and adjusts the drawer options again.

So,

1. Start without the tab present, and it's not present on the drawer
2. Add the tab, and it's not present on the drawer
3. Remove the tab, and now it's present on the drawer.

Edit:

Fixed with https://github.com/nikclayton/Tusky/commit/d2dfe8e8f57e024577ac42c18567c2190b5e58e4, which you'll want to merge in.

[nikclayton]

@DavidEdwards Haven't heard from you, and we wanted to move forward with this, so I've rolled everything up in to #3315 and in to a fully mergeable state.

If you have the time, or disagree with any of the feedback and want to continue on with this PR please say so.

[DavidEdwards]

It has been a busy couple of weeks. I'll make some time Monday evening to look things over.

[DavidEdwards]

@nikclayton It looks good. I like how it has turned out, especially the bezier curves.

Should I close this PR?

[nikclayton]

@nikclayton It looks good. I like how it has turned out, especially the bezier curves.

Should I close this PR?

Depends how you feel about attribution in GitHub I think.

1. If you merge my branch in to yours (in this PR), and this PR gets submitted that shows as a contribution from you.

2. If 3315 is merged I think it shows as a contribution from me.

My preference is #1, you've done the bulk of the work, I just tidied up a few things.

But if you haven't got time to do the merging, 3315 is ready for merging now.

[DavidEdwards]

Then I will merge it into my branch during my lunch today. Should be within the next hour or so.

[DavidEdwards]

@nikclayton Should be good. Thank you for the improvements 💯