Skip to content

feat: Put option to hide self-boosts in preference (#39) - #193

Merged
nikclayton merged 4 commits into
pachli:mainfrom
sanao1006:39-hide_self_boosts
Oct 23, 2023
Merged

nikclayton merged 4 commits into
pachli:mainfrom
sanao1006:39-hide_self_boosts

Conversation

@sanao1006

Copy link
Copy Markdown
Contributor

Overview

Some Mastodon posters have a annoying habit of boosting their own posts some time after they've posted them.
Some people got fed up , TL became disorganized due to this.

No need to see the same toot over and over again.

Changes Made

  • Add an additional option to the "Filters > Tabs" preference to show these self-boosts (default: on)

Screenshot

screen of "Filters > Tabs" preference

before after
image image

screen of Home Timeline

switch-on(self-boosts are displayed) swith-off(self-boosts are not displayed)
image image

Concern

With the current implementation, when "Show boosts" is OFF, boosts are not displayed whether "Show self-boosts" is ON or OFF. Is this behavior OK?

Related issue

Fixes #39

Some Mastodon posters have a annoying habit of boosting their own posts some time after they've posted them.
No need to see the same toot over and over again.

Therefore, add an option to show these self-boosts in the preferences (default: on).

Fixes pachli#39
@sanao1006 sanao1006 changed the title feat: Implementation of feature to hide self-boosts (#39) feat: Put option to hide self-boosts in preference (#39) Oct 22, 2023
(status.reblog != null && filterRemoveReblogs)
(status.reblog != null && filterRemoveReblogs) ||
// To determine if the boost is boosting your own toot
((status.reblog != null && status.account.username == statusViewData.username) && filterRemoveSelfReblogs)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

username is used to determine if the person who tooted and the person who tooted the boosted toot are the same. For example

@sanao1006@mastodon.social == @sanao1006@mastodon.social
// → true

However, this approach seems a bit un-smart. Any other good ideas (e.g., using id)?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, you're right, that could be a problem, because username doesn't include the domain (https://docs.joinmastodon.org/entities/Account/#username) so if user @foo@server1.example reblogs a post from @foo@server2.example this code would consider that to be a self-boost, because the username parts are the same.

As you say, it would be better to use id here.

Something like:

(filterRemoveSelfReblogs && (status.account.id == status.reblog?.account?.id))

should do it (untested).

@sanao1006
sanao1006 marked this pull request as ready for review October 22, 2023 05:06
@nikclayton
nikclayton self-requested a review October 22, 2023 12:46

@nikclayton nikclayton left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for contributing this. I've attached some suggestions.

switchPreference {
setTitle(R.string.pref_title_show_self_boosts)
setSummary(R.string.pref_title_show_self_boosts_description)
key = PrefKeys.TAB_FILTER_HOME_SELF_BOOSTS

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
key = PrefKeys.TAB_FILTER_HOME_SELF_BOOSTS
key = PrefKeys.TAB_SHOW_HOME_SELF_BOOSTS

I appreciate that this is going to be different from the other filter options, but please could you rename this to TAB_SHOW_HOME_SELF_BOOSTS (and change the string to that's used as the underlying preference key).

This makes the value of the preference (true or false) match what the prefkey name actually means. Right now, with "filter" in the name it's reasonable to expect that the value being true means the filter is applied.

TAB_FILTER_HOME_BOOSTS and TAB_FILTER_HOME_REPLIES get this wrong, and I'd prefer not to repeat the error.

You can see this elsewhere in the new code where you have:

filterRemoveSelfReblogs =
                !sharedPreferencesRepository.getBoolean(PrefKeys.TAB_FILTER_HOME_SELF_BOOSTS, true)

This is the correct code, but it looks like a bug because the value of the preference is negated before assigning to filterRemoveSelfReblogs, and the pref key and variable both have "filter" in the name.

If that was:

filterRemoveSelfReblogs =
                !sharedPreferencesRepository.getBoolean(PrefKeys.TAB_SHOW_HOME_SELF_BOOSTS, true)

that looks correct -- the "show" value is being negated to create a "filter" value.

I'm going to do a revamp of the preferences (basically, remove all use of the getBoolean and similar functions, and expose properties in SharedPreferencesRepository) which will fix this properly, but as I say, I'd prefer not to continue the problem in new code.

key = PrefKeys.TAB_FILTER_HOME_SELF_BOOSTS
setDefaultValue(true)
isIconSpaceReserved = false
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Untested, but if you put

dependency = PrefKeys.TAB_FILTER_HOME_BOOSTS

here then the new preference should be disabled when the "show boosts" preference is disabled (see the description in https://developer.android.com/develop/ui/views/components/settings/components-and-attributes).

This should make it clear to the user that if they disabled all boosts then the "disable self boosts" setting is ignored.

(status.reblog != null && filterRemoveReblogs)
(status.reblog != null && filterRemoveReblogs) ||
// To determine if the boost is boosting your own toot
((status.reblog != null && status.account.username == statusViewData.username) && filterRemoveSelfReblogs)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, you're right, that could be a problem, because username doesn't include the domain (https://docs.joinmastodon.org/entities/Account/#username) so if user @foo@server1.example reblogs a post from @foo@server2.example this code would consider that to be a self-boost, because the username parts are the same.

As you say, it would be better to use id here.

Something like:

(filterRemoveSelfReblogs && (status.account.id == status.reblog?.account?.id))

should do it (untested).


const val TAB_FILTER_HOME_REPLIES = "tabFilterHomeReplies_v2" // This was changed once to reset an unintentionally set default.
const val TAB_FILTER_HOME_BOOSTS = "tabFilterHomeBoosts"
const val TAB_FILTER_HOME_SELF_BOOSTS = "tabFilterHomeSelfBoosts"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
const val TAB_FILTER_HOME_SELF_BOOSTS = "tabFilterHomeSelfBoosts"
const val TAB_SHOW_HOME_SELF_BOOSTS = "tabShowHomeSelfBoosts"

Per earlier comment.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Per earlier suggestion.

reloadKeepingReadingPosition()
}
}
PrefKeys.TAB_FILTER_HOME_SELF_BOOSTS -> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
PrefKeys.TAB_FILTER_HOME_SELF_BOOSTS -> {
PrefKeys.TAB_SHOW_HOME_SELF_BOOSTS -> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Per earlier suggestion.

}
}
PrefKeys.TAB_FILTER_HOME_SELF_BOOSTS -> {
val filter = sharedPreferencesRepository.getBoolean(PrefKeys.TAB_FILTER_HOME_SELF_BOOSTS, true)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
val filter = sharedPreferencesRepository.getBoolean(PrefKeys.TAB_FILTER_HOME_SELF_BOOSTS, true)
val filter = sharedPreferencesRepository.getBoolean(PrefKeys.TAB_SHOW_HOME_SELF_BOOSTS, true)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Per earlier suggestion.

filterRemoveReblogs =
!sharedPreferencesRepository.getBoolean(PrefKeys.TAB_FILTER_HOME_BOOSTS, true)
filterRemoveSelfReblogs =
!sharedPreferencesRepository.getBoolean(PrefKeys.TAB_FILTER_HOME_SELF_BOOSTS, true)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
!sharedPreferencesRepository.getBoolean(PrefKeys.TAB_FILTER_HOME_SELF_BOOSTS, true)
!sharedPreferencesRepository.getBoolean(PrefKeys.TAB_SHOW_HOME_SELF_BOOSTS, true)

sanao1006 and others added 3 commits October 23, 2023 20:26
The previous code was incorrect in naming the preference key.
If the value of preference is a boolean value, it is reasonable to assume that the filter is applied if the value is true because the name contains "filter".

Therefore, change "filter" to "show" in the preference key name.

Fixes pachli#193

----------
Co-authored-by: Nik Clayton <nik@ngo.org.uk>
pachli#193)

The previous code used "username" to determine if the boosted toot and the user who boosted the toot were the same.
But this is incorrect because "username" does not include the domain and may not be unique.

Therefore, change the logic to make that determination using id.

Fixes pachli#193

----------
Co-authored-by: Nik Clayton <nik@ngo.org.uk>
… switch is off (pachli#193)

The previous code allowed the "Show self-boosts" switch to be toggled even when the "Show boosts" switch was off.
However, when the "Show boosts" switch is off, "Show self-boosts" must always be off.

Therefore, change it so that if the user disables all boosts, the "Disable self-boosts" setting is ignored.

Fixes pachli#193

----------
Co-authored-by: Nik Clayton <nik@ngo.org.uk>
@sanao1006

Copy link
Copy Markdown
Contributor Author

Behavior of the switchPreference

video.webm

setDefaultValue(true)
isIconSpaceReserved = false
}
}.apply { dependency = PrefKeys.TAB_FILTER_HOME_BOOSTS }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  ...
  setDefaultValue(true)
  isIconSpaceReserved = false
  dependency = PrefKeys.TAB_FILTER_HOME_BOOSTS
}

I really wanted to write the above code, but the following error occurred.

java.lang.IllegalStateException: Dependency "tabFilterHomeBoosts" not found for preference "tabShowHomeSelfBoosts" (title: "Show self-boosts"

I couldn't figure out how to solve this error and tried various ways, but when I used apply, it worked. So, I am sorry, but I have no evidence as to why it worked when I used apply...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Huh. That'll be something in the way SettingsDSL manages things. Not to worry, this is absolutely fine.

@sanao1006
sanao1006 requested a review from nikclayton October 23, 2023 13:06

@nikclayton nikclayton left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, thanks. I'll release a version of Pachli Current with this in the next hour or so, and this will be in 1.3 which I aim to have released on Wednesday.

setDefaultValue(true)
isIconSpaceReserved = false
}
}.apply { dependency = PrefKeys.TAB_FILTER_HOME_BOOSTS }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Huh. That'll be something in the way SettingsDSL manages things. Not to worry, this is absolutely fine.

@nikclayton
nikclayton merged commit 1f2f4d5 into pachli:main Oct 23, 2023
@sanao1006

Copy link
Copy Markdown
Contributor Author

Thanks for your review!

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.

Hide self-boosts

2 participants