Page MenuHomePhabricator

[L] Convert Tabs to use database, not shared preference.
Open, MediumPublic5 Estimated Story Points

Description

Our current implementation of Tabs uses a SharedPreference for storing the list of tabs (and the backstack for each tab), which is architecturally unsound and unsustainable.

This should be refactored to use a database table, similar to other features in the app that make use of arbitrary lists of items. After this is done, we can consider increasing the maximum number of tabs from the current arbitrary limit of 100. This will also resolve numerous downstream issues and other weird behavior when the number of tabs approaches or exceeds 100.

Event Timeline

Dbrant triaged this task as Medium priority.Dec 18 2023, 3:43 PM

Is Room database the best approach for storing Tab data (Tab as an entity) ? Are there alternatives or best approach ?

Is Room database the best approach for storing Tab data (Tab as an entity) ? Are there alternatives or best approach ?

Sure, since the rest of the project uses Room entities, this would make sense.

This seems like a big task for me as a newcomer. Perhaps I could start with a smaller issue to get acquainted with the codebase

Seddon renamed this task from Convert Tabs to use database, not shared preference. to [L] Convert Tabs to use database, not shared preference..Sep 12 2024, 5:31 PM
Seddon subscribed.

This task will not remove the current limit on the number of tabs. That will be handled in a seperate task.

Noting that so far cooltey estimates this has already been 9 points of work. And around 5 points amount of work remains.

Note on the current status of the work:

  1. Tab database table to store a list of backstack IDs.
  2. PageBackStack table to store the actual articles in the tab. The ID will be used in the Tab table.

Tabs screen

  • Migration completed.

Article screen

  • Upcoming refactoring needed:
    • Need to work on different tab action scenarios - open the article in: background tab, foreground tab, squash backstack items, current tab
    • Need to make sure only to query the current tab + backstack item for the article instead of keeping them in memory.
    • Need to make sure to handle the runBlocking vs proper coroutineScope for either tab count or tab commit actions.