neutrino icon indicating copy to clipboard operation
neutrino copied to clipboard

Table widget

Open alexislozano opened this issue 4 years ago • 7 comments

alexislozano avatar Sep 22 '19 15:09 alexislozano

Opinion: TableState should have ways to delete a certain row or even, all rows.

I am testing the Table widget to implement a file queue in my app.

Basically, I have a HashMap of files, and in TableListener::on_update() I'm simply updating the table to list all files in that map. Naturally, this map changes over time (and over user actions), but there is no way to delete row that correspond to files dropped from the map.

Adding TableState::remove_row(index) (and, optionally, TableState::remove_all_rows()) would suffice.

hyunh90 avatar Oct 18 '19 20:10 hyunh90

Additional discussion point

Currently, methods that modify the table headers/rows does not validate if the input matches with each others' column count. For example, make a table of 5 rows and 2 columns, then suddenly set the header to have just one column. Basically, the second column would be left without header and (depending on the backed browser engine) might clip out or ruin the table.

hyunh90 avatar Oct 19 '19 16:10 hyunh90

Yes, that is something I have seen, but I am not sure of how it should be handled here... Do you have an idea ?

alexislozano avatar Oct 19 '19 17:10 alexislozano

Since the table header is what "defines" (or describes?) the data of a table, matching each row's column count to header's column count seems logical in this case?

How about:

  • Manipulating headers upon building a Table widget via Table::set_headers => stay as-is
  • Manipulating headers in the middle via TableState::set_headers => empty entire row set as there is no guarantee those rows' column count matches with new headers => optionally give developers a choice where they can acknowledge the consequences and keep current rows content
  • Manipulating rows in any case =>validate if each row contains exact number of columns to header's (Might need to change method signatures to return Result)

For now, these are what floating in my mind.

hyunh90 avatar Oct 19 '19 18:10 hyunh90

If you want to try it feel free to do it and update your PR ;)

alexislozano avatar Oct 19 '19 22:10 alexislozano

During my attempt to implement the above logic, I realized that it's not an easy job to decide among these two possible ways:

  1. Make set_rows and add_row to return Result<(), TableError>:
    • Ok(()) when everything is sane
    • Err(TableError::HeaderRowColumnCountMismatch) when there is a column count discrepancy (i.e., header has n columns but the row has m columns)
    • Err(TableError::WithinRowsColumnCountMismatch) when rows have different column counts from each other (i.e., row x has n columns while row y has m columns)
  2. For each case of a row having less or more columns than the header or other rows, do:
    • fill with an empty string literal ("") if this row has less columns
    • Ignore excess columns if this row has more columns

Which one do you think the best?

hyunh90 avatar Oct 20 '19 02:10 hyunh90

I think that the best way would be to let the app dev to choose what to do when there is a discrepancy, thus the first one is, in my opinion, the best way. And thanks for trying solving this issue, it is amazing ! :D

alexislozano avatar Oct 20 '19 08:10 alexislozano