Customizable drawer

[DavidEdwards]

Add all options from customizing the Tabs to a new system from customizing the side drawer. This allows us to resolve requests such as #3010.

out.mp4

[connyduck]

I think this would be a nice addition, if you clean this PR up I'll review it!

[DavidEdwards]

Absolutely. Now the other is merged into develop, this one can be rebased and made cleaner.

I'll look into that soon.

[DavidEdwards]

@connyduck It should be runnable / testable / reviewable now. It is synced with develop.

I am leaving it on draft, as I am positive changes will need to be made.

[connyduck]

I think we should not have a special section in the drawer that can be customized, rather make everything above Account prefs/prefs/About customizable. We may need to have some items that can only be added to the drawer and not as tab, but most items should make sense as both.

[DavidEdwards]

@connyduck It should be in a good state now. Have a look when you have the time.

I just noticed the issue here too: #3331

@nikclayton You might want to have a look what you think too.

[nikclayton]

Will do, after I've wrapped up #3436

[connyduck]

I love this, really cool feature 😍

[connyduck]

This should definitely have a different name now, how about "AppScreen" or "ScreenData"?

[connyduck]

And I wonder if two classes would model things better here.
In the end, all screens can be shown in their own Activity, and some can also be shown as Tabs. So maybe like this

open class ScreenData(
    val id: String,
    @StringRes val text: Int,
    val icon: Icon,
    val arguments: List<String> = emptyList(),
    val intentAction: (Context, List<String>, Boolean) -> Intent,
    val title: (Context) -> String = { context -> context.getString(text) },
) {
    override fun equals(other: Any?): Boolean {
        if (this === other) return true
        if (javaClass != other?.javaClass) return false

        other as ScreenData

        if (id != other.id) return false
        if (arguments != other.arguments) return false

        return true
    }

    override fun hashCode(): Int {
        var result = id.hashCode()
        result = 31 * result + arguments.hashCode()
        return result
    }

    open fun withArguments(args: List<String>): ScreenData = ScreenData(
        id = id,
        text = text,
        icon = icon,
        arguments = args,
        intentAction = intentAction,
        title = title
    )
}


class TabScreenData(
    id: String,
    text: Int,
    icon: Icon,
    arguments: List<String> = emptyList(),
    val fragmentAction: (List<String>) -> Fragment,
    title: (Context) -> String = { context -> context.getString(text) },
) : ScreenData (
    id = id,
    text = text,
    icon = icon,
    arguments = arguments,
    intentAction = { context, tabData, accountLocked -> TabActivity.getIntent(context, id, accountLocked) },
    title = title
) {
    override fun withArguments(args: List<String>) = TabScreenData(
        id = id,
        text = text,
        icon = icon,
        arguments = args,
        fragmentAction = fragmentAction,
        title = title
    )
}

then have one createTabScreenDataFromId(...): TabScreenData and a createScreenDataFromId(...): ScreenData that calls the former, instead of AllowedContext check for the class, use intentAction whenever you need an intent, fragmentAction for Fragments, etc...

(But is works now and isn't super bad, better leave it if it is too complicated)

[DavidEdwards]

Definitely, I like the idea of ScreenData. TabData really makes no sense any more in the given context. I'll see how easy the change to inheritance is.

[DavidEdwards]

It took a bit to refactor it. There are a lot of files touched by this. I think it is better in the long run.

[DavidEdwards]

@connyduck Alright, the latest code changes should represent what you asked for.

[nikclayton]

Just looking at the UX, I haven't looked at the code yet. I'm coming at this from the assumption that the goal in #3331 is good.

First impressions:

• Federated timeline can probably be put back on the list of default tabs
• Navigation menu doesn't show all destinations by default:
  • Notifications
  • Local timeline
  • Direct messages / conversations
• There's still a limit of 5 tabs
• Unclear why there's a limit of 10 entries in the navigation drawer
• Unclear why some things can appear in the navigation drawer and tabs (e.g., notifications) and some things are drawer only (e.g., scheduled posts).

And I think the interaction model is still backwards. The user has to think about where the are first (navigation drawer or tabs), and what they want to see there second.

But I think it should be the other way around -- the user creates / selects and configures the timelines that they want to see, and then controls where those timelines are placed.

Nowithstanding that, I also noticed an odd gap (with a divider) in the navigation menu between the bottom of the account info and the "Edit profile" menu entry, see screenshot:

[DavidEdwards]

First impressions:

Federated timeline can probably be put back on the list of default tabs
Navigation menu doesn't show all destinations by default:
Notifications
Local timeline
Direct messages / conversations

I tried to leave the list as it was. I think I only added federated and trending as defaults. Though this would be easy to change.

There's still a limit of 5 tabs

I would prefer not to get involved in that for this PR. That could always come afterwards.

Unclear why there's a limit of 10 entries in the navigation drawer

Yes, that is a good question. I am not sure why there's a limit. It is probably because I copied the code over and just edited it larger. We could certainly remove the limit.

Unclear why some things can appear in the navigation drawer and tabs (e.g., notifications) and some things are drawer only (e.g., scheduled posts).

It is a technical limitation. Some systems are activities and some are fragments. If the system uses an activity, I would have to refactor it to use a fragment. That is far too complicated. The downside is that some systems can only be accessed through intents.

And I think the interaction model is still backwards. The user has to think about where the are first (navigation drawer or tabs), >and what they want to see there second.

But I think it should be the other way around -- the user creates / selects and configures the timelines that they want to see, and then controls where those timelines are placed.

That is a much bigger question. I'm not sure I can answer it.

Nowithstanding that, I also noticed an odd gap (with a divider) in the navigation menu between the bottom of the account info and the "Edit profile" menu entry, see screenshot:

Good spot. I have removed it.

[DavidEdwards]

To do: clicking a notification doesn't open notifications if it isn't on the tabs. It should also support the sidebar.

[nikclayton]

Still working on the 22 release, and the other arch changes, so a quick drive-by comment:

It is a technical limitation. Some systems are activities and some are fragments. If the system uses an activity, I would have to refactor it to use a fragment. That is far too complicated. The downside is that some systems can only be accessed through intents

Maybe it would be easier to tackle refactoring the existing activities that don't have fragments, and turn them in to an activity+fragment combo before going further with this work?

That is a much bigger question. I'm not sure I can answer it.

So maybe we can also get group consensus on the ideas in #3331 first? Once we've got consensus on a direction we can go from there. ?

[DavidEdwards]

@connyduck What should we do with this? It is assigned to you for architecture?

I would be OK with rebasing this onto the current develop branch, if there is hope of resurrecting its' cold dead corpse. 🧟

[connyduck]

Yes I know I said I'll check the code, but then things happened, sorry for that.

I think we can resolve all code related issues one way or another, but since I'm not making decisions here anymore, we should make sure the team actually wants this feature before we put anymore time into it.