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

Support the mastodon 4 filter api #3188

Merged
merged 14 commits into from Mar 11, 2023
Merged

Conversation

Tak
Copy link
Collaborator

@Tak Tak commented Jan 17, 2023

What's done:

  • Use the filter metadata attached to statuses when available
  • Support the warn filter action
  • Updated filter editing UI
  • Gracefully degrade to the old filter api

Outstanding:

  • The edit filter activity needs to be made scrollable
  • Cleanup in the timeline code
  • Probably UI tweaking

Screenshot_20230117-183520

Screenshot_20230117-183605

@nikclayton
Copy link
Contributor

First visual impressions:

  • Margin/padding is off, the edges of the views are all at the edge of the screen

  • Instead of "Title" for the filter name, how about "Filter name"

  • Font size is inconsistent

  • Instead of a list of keywords, how about using the same UI as when specifying hashtags in a tab, and use chips? If you're not sure which UI I mean, there's a screenshot in the unrelated PR Show a close button for hashtag tab chips #3140

  • "Context" could be "Where to apply filter?"

  • The list of contexts ("Home", etc) could have their icons present in the list for consistency with the main tabs, and the tabs list in other parts of the UI (e.g., the same screenshot from the PR I mentioned earlier).

Or, display them as filter chips (https://m3.material.io/components/chips/guidelines#8d453d50-8d8e-43aa-9ae3-87ed134d2e64).

  • Not sure what to do about the "Save" button at the bottom. On one hand, from the user's perspective this is part of the normal Preferences for the app, and it's weird to see a "Save" button in Preferences (unless it's on a dialog). On the other hand, they absolutely do need a clear and distinct way to distinguish between "I want to cancel the changes I am making", and "I want to be sure the changes I'm making are comitted". I'll have a think about this -- it might be as simple as using normal dialog style buttons (i.e., located at the bottom right, "Cancel" and "Save filter").

I'll mock up the UI changes directly so you can see what I mean, and send them as a patch.

@nikclayton
Copy link
Contributor

https://github.com/nikclayton/Tusky/commit/4a8c267ee88ab1ae70fd8eb3e6f0eb7db33f50c3 is a WIP that adjusts the padding and uses chips. There's still some layout stuff to do, and I haven't tried my other proposals yet because it's late here.

Here's a screenshot with 2 "whole word" keywords and one that isn't.

@nikclayton
Copy link
Contributor

Another thought -- the screen before this one (i.e., the one that lists all the filter names) is currently a list of single-line items.

That could probably be a list of two-line items, where the second line displays some key information about the filter, like what it does, duration, what it affects.

Or maybe three lines, and show some of the things it's filtering too?

Quick layout mock:

image

Not sure, to be honest, that might be overloading the user with too much information on that screen.

@Tak
Copy link
Collaborator Author

Tak commented Jan 18, 2023

WRT the labels, I based the layout and labels on the web UI - we probably shouldn't diverge too much from them on naming, or people will get confused about what thing in Tusky corresponds to what thing in the web UI.

Screenshot 2023-01-18 at 10 24 11

WRT the list, the web UI does have a richer list, but it also uses a lot of vertical space for each item, as well as assuming the availability of more horizontal space 🤷

Honestly, I'm not married to any of the UI, but the thing I couldn't actually get working was scrolling the edit activity

@nikclayton
Copy link
Contributor

I think the scrolling problem is because nothing was constrained to the bottom of the screen, so the layout is free to overflow the screen without needing to scroll.

I'll have a fix for that and some more layout stuff shortly.

@nikclayton
Copy link
Contributor

https://github.com/nikclayton/Tusky/commits/filter-chips has a new commit that does scrolling and a few other things.

There's full notes in the commit message. Latest screenshot:

In this example, the screen is scrollable, so the user can scroll up to set the rest of the contexts:

@digitalcircuit
Copy link

digitalcircuit commented Jan 20, 2023

Overall, this is excellent, thank you! I've been looking forward to this ever since Mastodon merged "filter as CW" into upstream.

@nikclayton

Or maybe three lines, and show some of the things it's filtering too?

Though it could be useful for some, I'd discourage implementing that.

Folks may set up filters to manage topics that they are currently negatively overwhelmed by (e.g. hate speech, politics, ex-friends/partners, etc), and if Tusky shows some of the actual filtered words in the "here's all your filters" list view, there's no way to view or edit any other filters without risking being reminded.

Instead, I'd suggest sticking with the two line summary. If a third line is needed, perhaps filter expiration (if set) and/or creation/edit date if available.

@Tak
Copy link
Collaborator Author

Tak commented Jan 25, 2023

I've applied most of nik's UI suggestions and cleaned up a bit

Screenshot_20230125-230836

@Tak Tak changed the title WIP: Support the mastodon 4 filter api Support the mastodon 4 filter api Jan 25, 2023
@Tak Tak marked this pull request as ready for review January 27, 2023 11:27
@Tak
Copy link
Collaborator Author

Tak commented Jan 27, 2023

I've also added more information to the filter list view

Screenshot 2023-01-27 at 13 33 35

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.

Very nice 😍

android:layout_weight="1"
android:fillViewport="true">

<androidx.appcompat.widget.LinearLayoutCompat
Copy link
Collaborator

Choose a reason for hiding this comment

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

LinearLayout everywhere please

(LinearLayoutCompat was relevant to access newer features on really old Androids, like 4.something)

@@ -59,7 +59,7 @@ class TimelinePagingAdapter(
}
else -> {
val view = LayoutInflater.from(viewGroup.context)
.inflate(R.layout.item_status, viewGroup, false)
.inflate(R.layout.item_status_wrapper, viewGroup, false)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't it be better to have another viewtype "VIEW_TYPE_FILTERED_STATUS" instead? (less stuff to inflate, less layouts hanging around at runtime)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🤔 If we do that, will we still be able to dynamically show the status when the user clicks "show anyway" ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think so

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, now I see what you mean - I completely misinterpreted what you meant before 🤦

import com.keylesspalace.tusky.util.getRelativeTimeSpanString

class FiltersAdapter(context: Context, val listener: FiltersListener, filters: List<Filter>) :
ArrayAdapter<Filter>(context, R.layout.item_removable, filters) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we use RecyclerView and RecyclerViewAdapter instead please? ListView/ArrayAdapter are really old apis and with RecyclerView we would also get nice animations when an element gets deleted.

@@ -203,6 +203,9 @@ AND timelineUserId = :accountId
)
abstract suspend fun deleteAllFromInstance(accountId: Long, instanceDomain: String)

@Query("UPDATE TimelineStatusEntity SET filtered = NULL WHERE (serverId = :statusId OR reblogServerId = :statusId)")
abstract suspend fun clearWarning(statusId: String): Int
Copy link
Collaborator

Choose a reason for hiding this comment

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

Needs to also check for the correct accountId as to not accidentally affect statuses from othr accounts with the same id.

import java.util.Date
import javax.inject.Inject

class EditFilterActivity : BaseActivity() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be nice to have ViewModels for this and FiltersActivity so they can handle screen rotation without reloading

@@ -0,0 +1,54 @@
<?xml version="1.0" encoding="utf-8"?>
<androidx.constraintlayout.widget.ConstraintLayout
Copy link
Collaborator

Choose a reason for hiding this comment

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

weird filename - maybe better "item_filter"?

}

companion object {
private const val TAG = "EditFilterActivity"
Copy link
Collaborator

Choose a reason for hiding this comment

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

unused


// Mastodon *stores* the absolute date in the filter,
// but create/edit take a number of seconds (relative to the time the operation is posted)
fun getSecondsForDurationIndex(index: Int, context: Context?, default: Date? = null): Int? {
Copy link
Collaborator

Choose a reason for hiding this comment

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

default is never set anywhere? Except in tests.

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 resolve the conflicts

Tak and others added 13 commits March 11, 2023 07:13
Nested scrolling views (e.g., an activity that scrolls with an embedded list
that also scrolls) can be difficult UI.

Since the list of contexts is fixed, replace it with a fixed collection of
switches, so there's no need to scroll the list.

Since the list of actions is only two (warn, hide), and are mutually
exclusive, replace the spinner with two radio buttons.

Use the accent colour and title styles on the different heading titles in
the layout, to match the presentation in Preferences.

Add an explicit "Cancel" button.

The layout is a straightforward LinearLayout, so use that instead of
ConstraintLayout, and remove some unncessary IDs.

Update EditFilterActivity to handle the new layout.
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.

None yet

4 participants