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

Experimental: Replace 'Clear' button with a 'Swap' button (between two presets) #3848

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

mcclure
Copy link
Collaborator

@mcclure mcclure commented Jul 13, 2023

I haven't really discussed this with anyone on the team and I am not expecting it to be merged. It is an experiment / request for comments.

What it does: It replaces the "Clear" button with a button (labeled, as appropriate, "More", "Less" or "Swap") that swaps your notification filter settings between two filter "slots".

In expectation most users would use this to show/hide favs and boosts, the default toggles between "follow requests hidden" and "follow requests, favs and boosts hidden".

Why?:

  • I never use the "Clear" button and, in fact, consider it a little dangerous.
  • I use the "Filter" switch button frequently but I usually only use it to toggle the "Boosts" and "Favs" displays.
  • I really like the super-accessible "Filter" dialog like Tusky has and I prefer having it to the less-flexible "Mentions" / " All" toggle in most social clients, so I want the UI to keep that "Filter" dialog always available.
  • And I think a setup where there were a variety of filter settings would be too complicated both to use and to code.

So, a swap button.

Known problems:

  • I think I messed something up in AppDatabase.java. Android Studio claims MIGRATION_51_52 is an unused variable, and in my one test (possibly invalid because I was upgrading from a jacked-up custom build) it claimed there was no upgrade path from like, 48 to 52.
  • I have not tested the upgrade path and even if it works I think it's non-ideal (currently it duplicates your current notification filters into both notification "slots"
  • I kinda feel like the "filters" button should be on the left and the "swap" button should be on the right but I haven't tested with this yet.
  • The button says "More" if the other filter has more members, "Less" if the other filter has fewer members, and "Swap" if they have an equal number. I don't know if in our style guide it should be swapped (IE, "Less" to mean we are CURRENTLY showing less) and I don't know if this is inconvenient in the case where they happen to have an equal number (maybe it could be "Swap >" or "Swap <").
  • When you change filters, the adjustment is not the most attractive animation. This wasn't a problem before because changing your filters always coincided with a dialog appearing/disappearing so your eyes didn't have time to really see what was happening.
  • Notification clear now completely unavailable (although I haven't deleted the code yet, just commented it out)
  • The variable names were kind of inconsistent and this makes it worse (lots of swapping between "notificationFilters" and "notificationsFilters" to express the same thing, figured rather than fixing it I should get consensus on which of those two to go with

@mcclure
Copy link
Collaborator Author

mcclure commented Jul 13, 2023

This is what it looks like

@mcclure
Copy link
Collaborator Author

mcclure commented Jul 13, 2023

Remaining ktLint failure is:

/home/runner/work/Tusky/Tusky/app/src/main/res/values/strings.xml:617: Error: The resource R.string.notifications_clear appears to be unused [UnusedResources]

But this should not be "fixed" unless it is determined Clear will not be re-inserted somewhere else.

@nikclayton
Copy link
Contributor

I was thinking about adjusting some of this UI, but not quite in the same way.

  1. "Clear" would move to the menu, with no possibility of appearing as an icon in the top bar.

Rationale: It's a very destructive operation, even with the warning. It's also not something I expect people do very often, so requiring some extra taps to get to it should be fine.

  1. Move "Filter" to the menu, with it preferentially appearing (if there's space) as an icon. Probably "Filter" or "Tune" from https://fonts.google.com/icons?icon.query=filter&icon.set=Material+Icons.

Rationale: Not a destructive operation, and one that people probably do more often, so making it an icon if possible makes sense.

This would remove the "Show Notifications filter" preference.


As an alternative to option (2), create secondary tabs (https://m3.material.io/components/tabs/guidelines) for each of the filter options, so the user could tap on them and see a tab with just mentions, or follows, or reblogs, etc. So not just the mentions/all distinction that you noted.

Pro: The user can switch faster (tapping a tab is a single action, whereas changing the active set of filters requires several taps)

Con: The user can't have a Notification timeline that contains a mix of different types, they're moving between different views of the timeline.

@Lakoja
Copy link
Collaborator

Lakoja commented Jul 21, 2023

I never use the "Clear" button and, in fact, consider it a little dangerous.

I agree with that.
Does it really delete all notifications (on the server) forever? I was afraid to find that out. :-)

2. Move "Filter" to the menu, with it preferentially appearing (if there's space) as an icon. Probably "Filter" or "Tune" from https://fonts.google.com/icons?icon.query=filter&icon.set=Material+Icons.

What (when) do you mean with "if there is space"?

Just a quick idea: I would expect that "normally" a user would not filter. So the whole "Filter / Clear" bar could be initially removed.
Also with something like "notification grouping" (#3452) this would be much less needed.

The filter item would move to the menu and IF there was something configured it would be shown as a icon (?) bar as it is now. You could switch it on and off there with a single tap. There could also be two filter setting entries in the menu (a bar has lots of room for mor than one filter slot).

@mcclure
Copy link
Collaborator Author

mcclure commented Jul 21, 2023

Does it really delete all notifications (on the server) forever? I was afraid to find that out. :-)

Yes.

@nikclayton
Copy link
Contributor

@Lakoja

What (when) do you mean with "if there is space"?

See https://github.com/tuskyapp/Tusky/pull/3877/files#diff-a7cd8865d84230ab68f69e1f5c98738030c3a548fb9cec9ae2365ad2196ec32d and "ifRoom" option.

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.

3 participants