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

Iceshrimp improvements #516

Open
wants to merge 34 commits into
base: master
Choose a base branch
from

Conversation

Jacocococo
Copy link

@Jacocococo Jacocococo commented Aug 22, 2024

Port of sk22#1000. The rest of this text is mostly just copied from there.

Lots of improvements to make the experience better on Iceshrimp. These include:

  • Improvements to emoji reactions
    • Enable reactions by default
    • Synchronize favorites and reacts (Except for on Iceshrimp.NET)
    • Respect max amount of reactions on a post (works for Glitch-Soc too)
    • Disable reaction buttons for remote reactions
    • Place the UI's reaction in the same place as the server has placed it
    • Fix bug where a new Unicode reaction wouldn't show up in the UI before refresh if the status previously had no reactions (not specific to Iceshrimp)
  • Improved quote posts support
    • It's now actually possible to quote a post from Iceshrimp
    • Adds another check to remove the RE: text
    • Fixes crash when viewing a quote post
    • Shows notification of the user's post getting quoted properly
  • Improved announcement display
    • Don't show emoji reactions
    • Correctly show edit time
  • Properly use content types since only MFM is supported
    • Disable settings by default and hide them (users will have to log out and in again to disable the setting if they want to. It shouldn't really matter, but that could possibly solve a problem someone is having after updating)
    • Allow all content types to lower risk of unexpected crashes
    • Set MFM as the default content type
  • Hide unsupported features
    • Account notify (Except for on Iceshrimp.NET. Doesn't seem to be supported on the server yet but likely will in the future)
    • News discovery
    • Language selector (Except for on Iceshrimp.NET. Not sure if it works currently, but I at least expect it to do in the future)
    • Filter settings (Except for on Iceshrimp.NET)
  • Load instance info immediately on main thread instead of in the background (too useful for it to be worth the unnoticable difference in app opening time)
  • Fix constant app crash after being logged into an Iceshrimp account for some time (see 86b6adf)
  • Moshidon: Add proper padding between emoji reactions display and extended footer if show dividers is on

This information is very useful and it's loaded very quickly anyways
- Hide emoji reactions
- Correctly set editedAt
The constructor of AccountSessionManager may need this object which
could lead to a situation where it was being used before it had been
created.
Making it static and moving it to MastodonAPIRequest, the only place it
was being used anyways, seems to fix the issue.
This is specifically for tapping existing reactions on a post if the
user has already reached the instance max
We can be sure it doesn't support anything other than MFM
It's disabled anyways for new users, and allowing all lowers the chance
of crashes from a previously selected option being missing
Copy link

@FineFindus FineFindus left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.
I think it would be a good idea to add comments explaining some of the server-specific changes, e.g. //hide y because x doesn't support property z. Makes it easier to understand why the code is like this when reading it later :)

@LucasGGamerM
Copy link
Owner

I'm just thinking of how it's gonna be to merge this after I deal with the upstream merging 🤔

@Jacocococo
Copy link
Author

Jacocococo commented Aug 28, 2024

I'm just thinking of how it's gonna be to merge this after I deal with the upstream merging 🤔

I just tried pulling all of mastodon-android/master into my branch and none of my changes seemed to make the merging process noticeably more difficult (than it already is)

Close to all of the changes in this PR only build upon code that was already existent in Moshidon. More than a third of the changes are in a single file that doesn't even exist in mastodon-android (EmojiReactionsStatusDisplayItem). Almost this entire PR is simply making it so features already implemented to be used by i.e. Glitch or Akkoma instances also work well for Iceshrimp users

Iceshrimp.NET now also implements some APIs using the pleroma field, so
only checking for that would create false positives for Iceshrimp.NET
@LimePotato
Copy link

LimePotato commented Sep 15, 2024

would it be possible to use the mastodon post visibility picker over the pleroma one as well? local-only posting does not function due to moshidon detecting it as pleroma.

@Jacocococo
Copy link
Author

This would seemingly already have been fixed by a554059

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.

4 participants