Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Android app crashes when Table contains too many cells #1392

Open
t-arn opened this issue Dec 21, 2021 · 7 comments
Open

Android app crashes when Table contains too many cells #1392

t-arn opened this issue Dec 21, 2021 · 7 comments
Labels
android The issue relates to Android mobile support. bug A crash or error in behavior.

Comments

@t-arn
Copy link
Contributor

t-arn commented Dec 21, 2021

On Android, the app crashes when there is a Table that contains too many cells

To Reproduce
Steps to reproduce the behavior:

  1. Use the Toga example "table" as of [1198c0f]
  2. Create the Android build
  3. Run the example on the Android emulator

Expected behavior
The code should check if the max amount of cells would be exceeded (how much is this??)
If yes, the table should not be created and
an exception should be raised (is that what we want?)

Logcat

org.beeware.table D/Python: Running 'table' as __main__...
org.beeware.table W/g.beeware.table: type=1400 audit(0.0:202): avc: denied { read } for name="/" dev="dm-0" ino=2 scontext=u:r:untrusted_app:s0:c96,c256,c512,c768 tcontext=u:object_r:rootfs:s0 tclass=dir permissive=0
org.beeware.table W/g.beeware.tabl: Accessing hidden method Ljava/io/FileDescriptor;->setInt$(I)V (light greylist, JNI)
org.beeware.table I/Python: Python app launched & stored in Android Activity class
org.beeware.table I/Python: [Android] Not implemented: Widget.set_hidden()
org.beeware.table I/chatty: uid=10096(org.beeware.table) identical 1 line
org.beeware.table I/Python: [Android] Not implemented: Widget.set_hidden()
org.beeware.table I/Python: WARNING: Using empty string for missing value in data. Define a 'missing_value' on the table to silence this message
org.beeware.table W/System: A resource failed to call close. 
org.beeware.table I/Python: [Android] Not implemented: Widget.set_hidden()
org.beeware.table I/g.beeware.tabl: NativeAlloc concurrent copying GC freed 7313(541KB) AllocSpace objects, 0(0B) LOS objects, 20% free, 23MB/29MB, paused 295us total 102.449ms
org.beeware.table I/g.beeware.tabl: Background concurrent copying GC freed 16509(1382KB) AllocSpace objects, 0(0B) LOS objects, 17% free, 28MB/34MB, paused 328us total 136.771ms
org.beeware.table I/g.beeware.tabl: NativeAlloc concurrent copying GC freed 8091(728KB) AllocSpace objects, 0(0B) LOS objects, 16% free, 30MB/36MB, paused 287us total 145.263ms
org.beeware.table A/g.beeware.tabl: java_vm_ext.cc:641] JNI ERROR (app bug): global reference table overflow (max=51200)global reference table dump:

Environment:

  • Operating System: Android
  • Python version: 3.8 (but probably on all versions)
  • Software versions:
    • Briefcase: 0.3.5
    • Toga: 0.3.0.dev30 [1198c0f]

Additional context
Add any other context about the problem here.

@t-arn t-arn added the bug A crash or error in behavior. label Dec 21, 2021
@t-arn t-arn changed the title Android crashes when Table contains too many cells Android app crashes when Table contains too many cells Dec 21, 2021
@freakboy3742 freakboy3742 added up-for-grabs android The issue relates to Android mobile support. labels Mar 29, 2022
@samschott
Copy link
Member

Current Gtk and Cocoa implementations instantiate views only for the rows which are currently being displayed on screen. As the user scrolls through the table, they then reuse existing views (which scrolled out of view) after updating their content instead of creating new ones. This allows efficient resource usage and smooth performance even for a very large number of rows (~ 10,000).

It looks like Android supports something similar with RecyclerView. Using it would require subclassing to implement our own "table row" class with swappable content. I think it should be possible to use this together with a TableLayout.

@t-arn
Copy link
Contributor Author

t-arn commented May 2, 2022

Yes, RecyclerView should work:
https://github.com/monsterbrain/RecyclerviewTableViewAndroid

Currently, you can only implement subclasses in Android by extending the Android template. But @freakboy3742 does not like this approach, so we first need to extend rubicon.java so it supports subclassing
beeware/rubicon-java#25

@freakboy3742
Copy link
Member

An update on the current thinking about the Table widget:

What is needed here is a completely different approach - one that leans into Toga's stated goal of providing platform-appropriate representations for data, not literal interpretations like the current Android table.

"Excel-style" tabular data isn't something you see very often on phone apps. There's a very good reason the default Android (or iOS, for that matter) API doesn't have a "Table" widget, despite it being a very common desktop widget: traditional table presentation isn't a format well suited to the mobile form factor.

So - we should take a different approach. Instead of trying to force a "traditional" table into the mobile experience, we should have a different way of presenting tabular data on mobile.

The best candidate: a Detailed list with navigation pages.

When you add a Table to your layout, you would get a scrollable DetailedList that displays the 2 columns of data from the table, plus (optimally) an icon. By default, the icon and "primary" title will be the first column, and the "secondary" title will be the second column; but this should be configurable on the Table widget to select any column as the source of the primary, secondary, and icon.

When a user selects an item from the DetailedList, the app would navigate to a secondary page that shows all the detail from that line of the table. That navigation would have a "back" button in the header to go back to the previous page; any actions would also be exposed on the "detail" page (it may also be desirable to expose actions as swipe actions on the main DetailedList table view).

The configuration of primary, secondary and icon would be ignored on desktop platforms, and only used on mobile platforms.

This general approach would also allow the definition of Tree widgets - the same general approach would be used, but the navigation would be multiple pages, with each step in the navigation presenting an additional DetailedList of the sub-options, until you reach a leaf node in the tree.

The use of primary/secondary/icon definitions would also allow for a Tree widget on Winforms. The current limitation there is that trees can only have 1 column of data on Winforms. A clearly defined "primary" would specify the column that will be visible from the data source.

@t-arn
Copy link
Contributor Author

t-arn commented Feb 15, 2024

I converted my taApplister from using a tablebto using a DetailedList with secondary screen. This is the result:

Screenshot_20240215_124226_pyPlayground.jpg

Screenshot_20240215_124239_pyPlayground.jpg

@freakboy3742 Is this what you had in mind?

As for how to specify the columns for title and subtitle: Couldn't we just use the first and second accessor items? Then, by re-arranging the data and the accessors, the user could choose which colums to display in the DetailedList

@freakboy3742
Copy link
Member

There's a couple of minor details I might quibble about (e.g., the OK button at the bottom, rather than a back navigation arrow in the top bar), but yes - that looks broadly like what I had in mind.

As for what to display - yes, "first and second accessors" would definitely be a reasonable default. However, rearranging the columns won't always be viable - think of the case where on a macOS/GTK/Windows table, there's a column that makes the most sense as the last column, but it's important enough that it needs to be promoted to the subtitle for iOS/Android. My inclination is that Table would end up with a set of parameters to configure the title, subtitle and icon columns; these arguments would be ignored on desktop, but used to pick the appropriate column on mobile. This is essentially the same as what DetailedList does - pick which attributes of data will be used out of a data source - and should use the same format, if possible.

@t-arn
Copy link
Contributor Author

t-arn commented Feb 16, 2024

Is there already an API to set a back navigation arrow in the top bar to the left of the title?
As far as I know, the toolbar commands are all to right of the title.

@freakboy3742
Copy link
Member

There's no API for that; but adding a "NavigationView" to manage a "stack" of content is on the todo list - and would be the prerequisite for doing tree/table views on mobile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android The issue relates to Android mobile support. bug A crash or error in behavior.
Projects
None yet
Development

No branches or pull requests

3 participants