[Feature] Add support for collapsible statuses when they exceed 500 characters

[HellPie]

Description

Supports showing a button to collapse/expand long posts. Fixes #748.

Features

• Toggleable by the user
• Limit set to 500 characters (like Pleroma and other existing implementations)
• It works in:
  • Timelines
  • Notifications
• It works by:
  • Showing a custom button below the content
  • Hiding the button if the post is CW-ed until content is shown

Concerns

The way it was implemented has proven to be potentially expensive if more and more features will be added overtime to the timeline. This is mostly the fault of the current design within Tusky, which reloads layouts rather than just content.

A possible solution would be to redesign the core data models to store a full and a compressed version of the content but that would require extensive rewrite of several parts of the codebase which would not need to know about it, again, due to how Tusky's codebase is designed.

It has not proven to be an issue on my Nexus 6 (Lineage OS 15.1), OnePlus 3 (Lineage OS 15.1) and Moto G (1st gen, LineageOS 14.1) yet so I did not want to enforce the feature so deep into the internals of the application.

Screenshots

Video of the feature is available on Imgur, due to restrictions with file types allowed on GitHub.

[HellPie]

Commit a0a41ca and 0ee004d will be reverted to the original settings once this is approved, for as long as I'll have to work on it, I won't change those due to issue I had with Android Studio 3.1.4 not rendering previews for certain layouts.

Commit 8f6c327 may be reverted although it has proven to be very useful to me over the years and is suggested by the Android Developers website so I would discourage from doing so, while I would much rather encourage implementing it properly as a custom productFlavor as suggested by the article linked.

[charlag]

Hi! Thanks for this nice change.
If you could apply some cleanup, it will be even better.

I would also ask you to revert unrelated changes, just don't commit them or put them in your local stash. Even if they're useful, they should be in a separate PR.

[charlag]

There's one minor thing I noticed: when post is barely more than 500 symbols, the button just reveals some more symbols on the same line, making it useless.
Quick hack would be showing button for posts with 550+ symbols but cut the text to 500 or something like that

[HellPie]

I left some changes unresolved, awaiting further suggestions to avoid having to change the code again later.

[clarfonthey]

I'd personally like this configurable-- for example, cybre.space has 512 chars max and I'd rather not have posts with 512 characters get collapsed. Maybe 600 might be a good default to cover that case specifically.

[HellPie]

I was really hoping to not see that requested until after the merge, and I would like to get this to work at least in a very basic way to ship it and start getting bug reports before implementing extra code that will only turn out to make debugging person-specific and end up in some sort of bug micromanagement hell.

My ideal design for how this should work at its best possible state is:

• Collapse at 500 characters by default (although 500 is a lot on mobile, two posts can take up a screen)
• Allow some "runway" (10 characters in excess) to collapse after sentences, punctuation, words
• Do not collapse if the collapsed/visible ratio is not worth the effort (25% maybe)

And for the settings:

• Allow the user to toggle the feature entirely
• Allow the user to disable "smart collapse" and collapse the post at the exact character limit
• Allow the user to specify their own, preferred collapsible length

And then it would be "peak implementation" if we could:

• Get Gargron to aknowledge this damn feature as one, once and for all
• Get Pleroma and Mastodon to agree on a key-value in api/v1/instance like we have toot_max_length
• Change the "selection" process to: user settings, instance settings, 500

Now there are issues:
The current implementation is nearing the border of what I consider smooth performances, and that is what is stopping me from considering those as worth implementing into this single PR, without a wider userbase having tested the feature first.
I already explained my concerns in the PR description in the "Concerns" section, I want to make them clear again: adding those features will have a noticeable impact in performance, as of now the app is not going to die, it will run well with such a "simple" (imo it's already advanced compared to an MVP) implementation, but the cost in performance of adding everything else will require a deeper integration of the feature in the app.

[clarfonthey]

Ah, makes sense! I appreciate the thought you put into the design already. :)

Allowing runaway characters seems to fit the use case very well. In that case, config can come later. :)

[HellPie]

I also wanted to say I won't be able to fix anything during the weekend because I'll be out of town/region/lost in a forest. I'll try to finish implementing the proposed changes ASAP, which means probably next thursday.

[charlag]

No rush
I still don't see where performance bottleneck should be

[HellPie]

Sorry for that. I wanted to comment that I still need to fix things because that merge obviously didn't go as cleanly as GitHub mentioned and then Chrome decided to scroll the page after loading idk what. tl;dr: Misclick D: this is frustrating.

[charlag]

Besides some style issues I have nothing to add.
Sorry, I completely forgot about this.
I think this should be non-configurable, and afaik @connyduck agrees.
I did not run it yet, I will try to check it tomorrow but looks good code-wise, good job!

[HellPie]

If you change idea on the configurability please feel free to revert b5d13e but even if you don't plan to, I'd argue that asking the community about their preference regarding having or not a toggle in the settings fragment would be a much better way to understand if it's worth removing or if it's going to cripple user experience.
In my opinion the toggle is going to be the only part of the code that will be easy to maintain, that has near to no overhead from both a runtime and a developer's perspective and also it's probably the only UI element that looks exactly like you and @connyduck would like it to look, out of the box 😛

EDIT: @charlag since there was some discussion regarding changes you requested at the beginning and since they were a lot I ended up closing a few before commiting code and others we agreed on without needing to change code and probably something in there borked GitHub. I believe all changes you requested are actually fixed, although if you want to double check or reiterate something feel free to do that.

P.S.: I was kinda hoping for more community feedback since there seemed to be quite a lot of points, especially in the design, where me, you and conny aren't fully agreeing on, so idk if you want to trap card and "we can change it later" or if you want try and see what the community thinks before merging and shipping alphas because I don't really mind which decision you want to go for, I'm just worried that even if we all were to agree from the beginning on such a big change having 3 people's opinion and several thousands "blank cards" seems a bit risky imo, especially considering all the heat about "content warning"s, "longposting" and all the new Drama™ which makes features like this delicate changes.

[charlag]

That's the thing about both configurability and design: everyone has different opinions. We can ask, of course, but my guess is 90% of the people would agree with this being on. I see no reason to scroll through random 10k character post on mobile.
To get real UI feedback from a large crowd we need to publish it as nightly or even as a release.
I think making it a preference if there's a huge demand for it will be easier than removing some option (which is always a pain)

[charlag]

I've tested a bit and I think button either needs background or bold text or both but I'd like to hear others
Otherwise seems OK to me

[HellPie]

The button should be bold by default since buttons on Android are bold by default and I am not adding any style to the text whatsoever.
I will check on that, Idk about the background, I feel like it will mark it as the primary action of the content while I think the user should first read the first 500 characters and then notice the button if they want to continue, unlike the CW button which often only shows a 4 words extremely generic summary of the content and therefore should (and does) gain a predominant role in the UI of the post.

[charlag]

We discussed it with connyduck, we agree that the button should look like CW button. They promised to give more feedback tomorrow.
Thanks for your efforts! 🙇‍♂️

[cheesegrits]

Good work, looking forward to this feature.

I would argue for some configurability, even if it's just setting the cut off length globally. But I'm not religious about it, and it's definitely something that can be added later, after some user feedback.

Thanks for all the work y'all put into Tusky.

[connyduck]

I think its ready to merge now! Good job @HellPie

[HellPie]

Thanks @connyduck, spent quite some time on it, but it didn't turn out too hard to implement fortunately.

Not quite sure how to handle the Codacy complaint about complexity since they suggest splitting in more methods and that adds a lot of overhead for a method that must always work as fast as possible, also it wouldn't really fix the complaint from a practical point of view.

I fixed the import issue, that was quite critical imo since it's vital that if someone edits the code they won't pick up the wrong import statement since there are double APIs in place rn (bless fragmentation).

Also still waiting on @charlag to confirm all past changes are actually resolved, since they are still marked as requested on GitHub because I marked them manually rather than letting GitHub handle it through automation.

[connyduck]

oh please ignore the codacy thing, I enabled it by mistake

[HellPie]

ok, that import still bothered me tho so I'll keep that commit @connyduck
idk why now Circle is failing if before it worked, code looks fine, I'd also stick a "builds on my machine" badge. I'm investigating as we speak.

[connyduck]

I think its failing because its out of memory 🙄. Its fine as long as bitrise passes.

[HellPie]

I'll call it a day then, right on time before I had to leave, too, ty for the review btw

[charlag]

I can look tomorrow but I'm pretty sure it's okay if connyduck approved it. 16 Sep 2018, 21:34 by [email protected]:

…

Thanks > @connyduck <https://github.com/connyduck>> , spent quite some time on it, but it didn't turn out too hard to implement fortunately. Not quite sure how to handle the Codacy complaint about complexity since they suggest splitting in more methods and that adds a lot of overhead for a method that must always work as fast as possible, also it wouldn't really fix the complaint from a practical point of view. I fixed the import issue, that was quite critical imo since it's vital that if someone edits the code they won't pick up the wrong import statement since there are double APIs in place rn (bless fragmentation). Also still waiting on > @charlag <https://github.com/charlag>> to confirm all past changes are actually resolved, since they are still marked as requested on GitHub because I marked them manually rather than letting GitHub handle it through automation. — You are receiving this because you were mentioned. Reply to this email directly, > view it on GitHub <#825 (comment)>> , or > mute the thread <https://github.com/notifications/unsubscribe-auth/AC9KBmfie7lHn0zHsystKoR_na90gP67ks5ubqfagaJpZM4WU7wG>> .