Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(browser): render rows using backend #16620

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

david-allison
Copy link
Member

@david-allison david-allison commented Jun 19, 2024

Purpose / Description

Modify the CardBrowser to use a RecyclerView and use backend.browserRowForId to obtain the data

This is significantly faster, reducing or removing the 'jittery' rendering of the Card Browser

In addition, it means we can display the FSRS columns

Since the calls are so fast, we can perform them inside the UI thread

Fixes

Approach

  • BrowserRowCollection
    • (may want a better name)
    • The collection of rows returned to the Browser. This may EITHER be CardIds or NoteIds
  • backend.browserRowForId: returns BrowserRow
    • Accepts EITHER a CardId, or NoteId, depending on collection state
  • Modify searching to return a BrowserRowCollection
  • Modify rendering to handle BrowserRow
    • Using a RecyclerView

How Has This Been Tested?

Unit tests

  • Can someone give this a manual test with the FSRS columns in 'Notes' mode?
    • I recall there was a problem here and want to confirm it was fixed

Learning

  • Not a bug: Flag colors are not set in 'notes' mode
    • This matches Anki Desktop, but I spent time debugging it

Checklist

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

@david-allison david-allison added Help Wanted Requesting Pull Requests from volunteers Review High Priority Request for high priority review Blocked by dependency Currently blocked by some other dependent / related change labels Jun 19, 2024
@david-allison david-allison marked this pull request as draft June 19, 2024 18:12
Copy link
Contributor

Message to maintainers, this PR contains strings changes.

  1. Before merging this PR, it is best to run the "Sync Translations" GitHub action, then make and merge a PR from the i18n_sync branch to get translations cleaned out.
  2. Then merge this PR, and immediately do another translation PR so the huge change made by this PR's key changes are all by themselves.

Read more about updating strings on the wiki,

@david-allison david-allison force-pushed the card-browser-backend-render branch 2 times, most recently from a7260eb to 552596b Compare June 20, 2024 17:02
@david-allison david-allison added Needs Author Reply Waiting for a reply from the original author and removed Blocked by dependency Currently blocked by some other dependent / related change labels Jun 20, 2024
lukstbit

This comment was marked as resolved.

This comment was marked as outdated.

@github-actions github-actions bot added the Stale label Jul 6, 2024
@lukstbit lukstbit removed the Stale label Jul 8, 2024
@david-allison david-allison force-pushed the card-browser-backend-render branch 2 times, most recently from ae6184c to f8d07af Compare July 8, 2024 10:46
@david-allison david-allison removed Review High Priority Request for high priority review Has Conflicts labels Jul 8, 2024
@david-allison david-allison force-pushed the card-browser-backend-render branch 3 times, most recently from c38876d to d36e659 Compare July 10, 2024 16:31
@david-allison david-allison marked this pull request as ready for review July 10, 2024 16:32
@david-allison david-allison added Needs Review and removed Needs Author Reply Waiting for a reply from the original author Help Wanted Requesting Pull Requests from volunteers labels Jul 10, 2024
@david-allison david-allison marked this pull request as draft August 3, 2024 14:53
@david-allison

This comment was marked as resolved.

@david-allison david-allison force-pushed the card-browser-backend-render branch 2 times, most recently from b99c94f to 84e5c09 Compare August 3, 2024 15:44
@david-allison

This comment was marked as resolved.

@david-allison

This comment was marked as resolved.

@david-allison david-allison added the Needs Author Reply Waiting for a reply from the original author label Aug 5, 2024
@david-allison

This comment was marked as resolved.

@david-allison david-allison mentioned this pull request Aug 6, 2024
5 tasks
@david-allison david-allison added Blocked by dependency Currently blocked by some other dependent / related change and removed Needs Author Reply Waiting for a reply from the original author labels Aug 6, 2024
@david-allison
Copy link
Member Author

  • when long-pressing a row to enter multiselect mode, the view position of the selected row should not change

@david-allison david-allison removed the Blocked by dependency Currently blocked by some other dependent / related change label Aug 19, 2024
@david-allison
Copy link
Member Author

Up to other maintainers for guidance, but I'd prefer to focus efforts on

@david-allison david-allison marked this pull request as draft August 19, 2024 12:05
`browserRowForId` accepts either a `NoteId` or a `CardId`

depending on the value of `BROWSER_TABLE_SHOW_NOTES_MODE`

* CardOrNoteId
  * represents the ID of a Browser Row
* BrowserRowCollection
  * A collection of the browser rows, along with the context
  (CardsOrNotes) to know how to obtain the raw cids/nids from
  the collection
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Has Conflicts Needs Second Approval Has one approval, one more approval to merge Strings
Projects
None yet
4 participants