Skip to content
This repository has been archived by the owner on Jul 29, 2022. It is now read-only.

Use fewer activities in the testapp #352

Closed
stevenzeck opened this issue Aug 23, 2020 · 1 comment · Fixed by #403
Closed

Use fewer activities in the testapp #352

stevenzeck opened this issue Aug 23, 2020 · 1 comment · Fixed by #403

Comments

@stevenzeck
Copy link
Contributor

I know this is just a testapp, but there are a lot of activities used here. I think several of them could be converted to fragments, while also using a navigation graph. My initial thoughts:

  1. In theory, what is in R2DispatcherActivity could just be put into CatalogActivity
  2. R2AboutActivity should just be a fragment
  3. For the OPDS related activities (OPDSListActivity, OPDSDetailActivity and OPDSCatalogActivity, maybe do a bottom navigation? One for catalog related stuff and the other for OPDS?
  4. Kind of going off of RC 1.2.0 r2-navigator-kotlin#143 and Exposing Fragment instead of Activity kotlin-toolkit#176, have one activity for reading instead of the three now (EpubActivity, DiViNaActivity and ComicActivity), and use the navigators fragments inside that activity. I don't know if the navigator is ready to do that yet.
@mickael-menu
Copy link
Member

The test app was designed a few years ago, but since then the Android architecture landscape changed a lot (for the better!), thanks to Google's Architecture Components. So as you noticed, there's a lot of opportunity for modernization in the test app. For now our priority is to modernize the toolkit itself, and once this is done we'll tackle the test app. But you're welcome to address some of the issues in the test app if you want 👍

Ideally, we'll end up with a test app that is also a good example of modern architecture practices, to set implementers for success right from the start.

Here's a few other things I have in mind:

  • Merge the different database files into a single one, and use Room instead of the raw SQLite library.
  • Add a proper domain layer to manage user publications and annotations.
  • Add a service layer to handle stuff like importing publications.

I think doing these first would greatly simplify the UX layer and so any UX refactoring, by moving any business logic from the Activity to services or ViewModel instances.

Using a navigation graph (or several, I think the OPDS module could benefit from being more independent and owning its own graph) is a good idea as well.

  1. For the OPDS related activities (OPDSListActivity, OPDSDetailActivity and OPDSCatalogActivity, maybe do a bottom navigation? One for catalog related stuff and the other for OPDS?

What other stuff do you have in mind? Right now for the catalogs screen we only have a + button to add more catalogs. However, I think a bottom navigation would make sense in the library screen, to split it between "Bookshelf" and "Catalogs".

  1. Kind of going off of RC 1.2.0 r2-navigator-kotlin#143 and Exposing Fragment instead of Activity kotlin-toolkit#176, have one activity for reading instead of the three now (EpubActivity, DiViNaActivity and ComicActivity), and use the navigators fragments inside that activity. I don't know if the navigator is ready to do that yet.

The navigator is not really ready for that yet, but yeah this is the end goal. Having a single Reader activity (or maybe fragment) which is mostly format-agnostic and relies on the Navigator interfaces for stuff like saving the current locator, to factorize code.

Also we should keep in mind that developers might be interested in only a subset of what the test app offers, so we should clearly segregate the bookshelf, catalogs and reader modules.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants