Skip to content

Instantly share code, notes, and snippets.

@nikclayton
Last active April 30, 2024 14:58
Show Gist options
  • Save nikclayton/34710790154b06c8ceb3cf9efb6913f8 to your computer and use it in GitHub Desktop.
Save nikclayton/34710790154b06c8ceb3cf9efb6913f8 to your computer and use it in GitHub Desktop.
Semi-structured work plan.md

This is a very rough list of things to do in Pachli. Some of these are already filed as issues that may have more information, others aren't because I'm trying not to overwhelm the issue list with work that might be several months away.

This list is likely incomplete. See Issues · pachli/pachli-android · GitHub for more.

Finish code modularisation

What and why: Finish the work of splitting the code in to modules:

core:... modules with core functionality feature:... modules corresponding to a single feature / set of screens app that ties it together

Benefits include:

  • Faster build times, locally and on CI
  • Modularisation is forcing function to improve code architecture

Downside is it becomes a bit more difficult to navigate around the code -- chiefly because there's a lot of scrolling in the project navigator. But Android Studio Ctrl-B, Ctrl-N, etc, jump straight to definitions / declarations which works well.

Who: Nik

State: In-progress

Good for contributors: Probably not; there's quite a lot of state to hold in your head during the refactoring, and other changes can be very disruptive. I've got the time to do that, others might not. If you do want to modularise a chunk of code get in touch first.

Unlocks:

Links:

Finish architecture refactorings

What and why: Finish the work of refactoring the code's architecture ("finish" is aspirational, there's always likely to be improvements).

The current code architecture is a wildly divergent mix of styles. It's converging on a clear (high-level)

Activity/Fragment -> ViewModel -> Repository

structure, with the UI level sending actions to the viewmodel and the viewmodel sending state back, but there's a lot of code that doesn't do this yet (e.g., API calls from within the activity/fragment or viewmodel, no repository, unclear state communication from the viewmodel to its activity/fragment, etc).

ComposeActivity and its hierarchy is a good example of the current mess.

Cleaner architecture means fewer places for bugs to hide. Better consistency in structuring the code makes it easier to navigate between and understand different parts of the app.

See [[Code smell auditing]] for more notes.

Who: Nik

State: In-progress

Good for contributors: Maybe... I don't think there are enough good examples of "Structure it like this" that I can point to that would make it easy for another contributor to go off and do the work without risking a PR that would get a lot of comments.

Unlocks:

Links:

Screenshot tests

What and why: Integration tests that drive the app, take screenshots at various points, and compare them against "golden master" screenshots as part of the PR review process.

  1. Catches UX regressions before they make it to users
  2. Easier to review PRs that contain UX changes; reviewer can check screenshots, doesn't need to build PR locally
  3. Going to be essential before expanding support to tablets and other large devices

Some concerns:

  • Screenshot tests, if not done well, can be quite flaky. The process of making sure that sources of flakiness are removed (e.g., hiding the cursor) can be quite involved.
  • Screenshot tests might take a significant amount of time to run on CI. Needs testing
  • Differences between Linux, Mac, and Windows can mean screenshot tests produce different screenshots in their respective emulators. This makes them not quite as useful during local development unless the local development environment matches CI.

Who:

State: Not started

Good for contributors: Yes, especially if they've got prior experience.

Unlocks:

  • [[Migrate to sp unit]]
  • [[#Design system]]

Links:

Design system

What and why: The current layouts don't follow a consistent grid design (margins, padding, heights, etc), and new layouts tend to be created by copying and pasting existing layouts. This makes it difficult to make cross-app changes to the layout (e.g., ensuring text views have a consistent accessible height everywhere) and complicates PR review because the design needs to be checked manually.

A design system with design tokens for common items, coupled with lint checks to ensure they're used would catch errors before they reach users, simplify PR review, and make it less likely that accessibility mistakes are made.

Needs [[#Screenshot tests]] first I think, so we can be confident that adapting a layout to use the design system doesn't break it

Who:

State:

Good for contributors:

Unlocks:

Links:

"Wide" view

What and why: People want a view variant where the space on the left side below the avatar is removed and the text content appears there.

No reason not to do it, except it complicates the code and the layouts. Much easier to be confident that this is done properly with a [[#Design system]] and [[#Screenshot tests]].

Who:

State:

Good for contributors:

Unlocks:

Links:

Review / refactor notifications

What and why: Review the notification code, fix issues

Notifications feel like a mess. Problems include:

  • Not cached
  • Not accessed through a repository
  • Unclear if errors registering with the push distributor are handled properly
  • Poll-notifications should prompt user to disable battery optimisations
  • Could prompt user to install a push distributor if one isn't present

Note to self: Also see [[Notifications investigation]].

Who:

State:

Good for contributors: Yes. Probably starts with a review of the existing code and a more complete assessment of the problems described above (as well as any others), then an outline for how to tackle them

Unlocks:

Links:

Rewrite AccountManager / account handling

What and why: The existing AccountManager class has a number of problems that need fixing.

It's possible for AccountManager.activeAccount to be null. This makes it harder to reason about existing code (is activeAccount legitimately non-null in all places it's asserted not to be, for example). It never should be when the user is outside LoginActivity but I don't completely trust that.

Data that is also part of the logged in account (e.g., lists the user has created) is also scattered through the app in different repositories / viewmodels / activities.

I think the solution looks something like the following:

  1. A new AccountRepository or similar that almost the entire app uses. There is always a non-null active account exposed by this repository.
    1. The account exposed is not AccountEntity (the database / DAO type) but a new PachliAccount. As well as the existing account data this has properties for things like lists, timelines, account preferences, etc.
    2. There'll be a AccountRepository.logout() method that logs out the current account. But that will need to take a lambda to execute if this logs out the last account (i.e., there will be no active account). This lambda should start LoginActivity
  2. LoginActivity is the only thing that has to handle the possibility of there being no account. That probably doesn't interact with any instances of AccountRepository (otherwise you have to deal with "How do you create AccountRepository with a non-null account before the user has logged in for the first time).
    1. AccountRepository probably provides a static createAccountAndSetActive() method that LoginActivity can use.

Who:

State:

Good for contributors: Probably not, as it touches approximately everywhere in the codebase.

Unlocks:

Links:

Include account information in all API calls

What and why: Most API calls at the moment use the ambient account.

Specifically, InstanceSwitchAuthInterceptor intercepts network requests, and uses whatever the current credentials are in a request.

AccountManager changes those credentials whenever the active account is changed.

I'm concerned there's a race condition there; the user starts an action (e.g., boosting a post) signed in with one account. Then they switch accounts and the network request hasn't happened yet (maybe device is still connecting to the network?). Then the request goes out, but now it's going to use different credentials.

Who:

State: Not started

Good for contributors: Yes. Modifying the API interface so every call requires an account, and then passing that account down through the code is a reasonable amount of work, but it's relatively straightforward -- once you've changed the API the compiler will tell you every call site that needs changing.

Unlocks: "Boost as X" functionality; i.e., being logged in with one account, but able to (e.g., on a long-press) perform an action (like a boost) on a post using one of your other accounts.

Links:

Better user onboarding / UI hints

What and why: As users go through the app they should get UI hints that point out things they might not know.

There are probably two types of hints to show:

  1. A multi-page onboarding flow when people use the app for the first time (sign in with their first account). This can prompt them to do things like set a font and size, colour scheme, CW preferences, ask for necessary permissions, etc).
  2. Contextual hints when the user does something for the first time. For example:
    1. When the user composes a reply, show popups that explain things like how to set the CW, how to see the post they're replying to, how to set the visibility, schedule it, etc.
    2. When the user opens a timeline from the left-nav menu, show them how to add that timeline to a tab.

Every hint should have an option to disable hints entirely, and there should be UI somewhere (probably in preferences) that allows the user to see all the hints so they can review them.

See [[Ideas for onboarding flow]].

Once looks like a promising library for managing the "Only show things to the user once" side of things.

Possible libraries to investigate (there may be more) for the contextual hints include:

Onboarding library links / inspiration include:

Who:

State:

Good for contributors: Yes. Want to sketch out the flow with them before receiving PRs, but this feels self contained, and once the first set of contextual hints are written it should be straightforward to add more.

Unlocks:

Links:

Investigate Jetpack Compose

What and why: One one hand, at some point we're probably going to have to migrate to Compose instead of XML views. It's the direction Google is pushing in, and some playing around with toy apps that use Compose does seem to show a nice integration with Android Studio -- building the UIs did seem easier than with XML views, and without the need to launch every change on a device to make sure it does what you want.

On the other hand, my unscientific "vibe" about Compose is that it's not ready yet. Every week I seem to see different blog posts of the form "Here's how you can finally do this thing in Compose that we've been able to do with XML layouts for years", and I don't want to have a long period of time where we're supporting XML and Compose within the same app module.

There also seem to be many subtleties around the remember concept that make it very easy to create a layout that's very inefficient to draw / update. I'm not sure if there's good tooling (e.g., lint) to catch this.

As the app becomes more modular I am open to the idea of a single module experimenting with Compose. Maybe feature:about for example. Have a PR that cuts that over to Compose and see what the problems are as well as the benefits, then make a decision about if / when / how to bring it to the rest of the app.

Who:

State:

Good for contributors: If they already have experience with Compose, and can talk about the pros/cons.

Unlocks:

Links:

New artwork

What and why: The logo's growing on me. I'm not averse to changing it in the future, but I am very attached to the visual gag of a stylised "P" looking like a left-facing elephant's head, and want to keep that, as well as the core colours, which are:

  - Pale grey, d4d2c6   - Blue, 006382   - Dark grey, 30383a   - Orange, f37b2f, S 1070-Y50R

The errorphant images are held over from Tusky and should be replaced as part of updating the visual identity.

Who:

State:

Good for contributors:

Unlocks:

Links:


Template

What and why:

Who:

State:

Good for contributors:

Unlocks:

Links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment