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

[WIP] Inline reply/boost/favourite counters #1369

Closed
wants to merge 8 commits into from

Conversation

noln
Copy link

@noln noln commented Jul 5, 2019

Adds counters next to reply, boost and favourite icons on posts.

As8KAjyjEy

Copy link
Collaborator

@Tak Tak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@connyduck mentioned that he'd like to see a preference to control this - is there a plan for that?

countReposts.setText(status.getReblogsCount()>0?String.format(Locale.getDefault(),"%d",status.getReblogsCount()):"");

if (countReplies!=null)
countReplies.setText(status.getRepliesCount()>0?String.format(Locale.getDefault(),"%d",status.getRepliesCount()):"");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The web UI does "0", "1", "1+" for replies - do we want to do the same?
What's the plan for dealing with many-digit counts on these?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how it works in the Roma fork so carried over the functionality, but open to suggestions.

@noln noln changed the title Inline reply/boost/favourite counters [WIP] Inline reply/boost/favourite counters Jul 6, 2019
@noln
Copy link
Author

noln commented Jul 6, 2019

@Tak it's always-on in this implementation, but could be switchable in the view layer if that's something that @connyduck is firm on. I've WIP'ed the PR pending feedback.

@JuanjoSalvador
Copy link
Contributor

That's pretty cool!

@charlag
Copy link
Collaborator

charlag commented Jul 7, 2019

I think we already had such a PR before?
#885
For the context

Copy link
Collaborator

@connyduck connyduck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • please add an option to configure this behavior. Default off.
  • The numbers next to the icons should have the same color as the icons. Looks way better imo.
  • when I boost a status, the number disappears and reappears when unboosting. Liking works fine
  • In detail view the like/boost count is now shown twice. I think its better to add the reply count to where we already show the like/boost count instead of showing like/boost twice

public void migrate(@NonNull SupportSQLiteDatabase database) {
database.execSQL("ALTER TABLE `ConversationEntity` ADD COLUMN `s_repliesCount` INTEGER NOT NULL DEFAULT 0");
}
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please merge these two Migrations into one?

@@ -612,32 +628,63 @@ private void handleStatusComposedEvent(StatusComposedEvent event) {

if (eventStatus.getInReplyToId().equals(thisThreadsStatusId)) {
insertStatus(eventStatus, statuses.size());
addReplyCount(0, 1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a bug? The detailed status is not guaranteed to be the first one in the list

countReposts.setText(status.getReblogsCount()>0?String.format(Locale.getDefault(),"%d",status.getReblogsCount()):"");

if (countReplies!=null)
countReplies.setText(status.getRepliesCount()>0?String.format(Locale.getDefault(),"%d",status.getRepliesCount()):"");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use numberFormat.format elsewhere, i prefer that
I actually like showing the nuber more than "1" "1+" .

joelpyska and others added 8 commits July 14, 2019 08:57
# Conflicts:
#	app/src/main/java/com/keylesspalace/tusky/appstore/CacheUpdater.kt
#	app/src/main/java/com/keylesspalace/tusky/appstore/Events.kt
#	app/src/main/java/com/keylesspalace/tusky/fragment/NotificationsFragment.java
#	app/src/main/java/com/keylesspalace/tusky/fragment/ViewThreadFragment.java
#	app/src/main/java/com/keylesspalace/tusky/network/TimelineCases.kt
# Conflicts:
#	app/src/main/java/com/keylesspalace/tusky/fragment/NotificationsFragment.java
#	app/src/main/java/com/keylesspalace/tusky/fragment/ViewThreadFragment.java
Added `s_repliesCount` column to ConversationEntity table, with corresponding version bump and schema. Required by the reply count part of the `counters` feature.
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.

6 participants