APrioriInvestments / object_database

A distributed object model written on top of typed_python, with a reactive web framework.
Apache License 2.0
5 stars 1 forks source link

"Table" cell no longer shows the correct page number #17

Open braxtonmckee opened 5 years ago

braxtonmckee commented 5 years ago

We're supposed to see an indicator of what page we're on in a multi-page Table, and to be able to edit it in a textbox. We seem to have lost this behavior during refactoring.

dkrasner commented 5 years ago

can you show a screenshot of a working version?

braxtonmckee commented 5 years ago

image

braxtonmckee commented 5 years ago

The idea is you have the arrows to switch pages and a text box to type the row number if you want.

darth-cheney commented 5 years ago

Do you have an example of how we can build a multi-page table, for the playground? I'd like to spin something up where I can test this live

braxtonmckee commented 5 years ago

see d78ac0988d50eb9bf6e80398b07a62587ba3d1f2

darth-cheney commented 4 years ago

Hi @atzannes and @braxtonmckee, When you guys have a minute, can you pull down the eric-issue-17-rebased branch and test out the Table functionality? This is a version of the fix in PR #50 that I have rebased on top of dev as of this writing. I also added some automated CellsDemo tests to ensure that pagination work.

The thing I'd really like you both to try out is the new-style filtration in the table header. Let me know what you think and if you find any issues.

atzannes commented 4 years ago

Hi @darth-cheney apologies for the delay. I checked out your branch and played with the MultiPageTable. I added some minor suggestions to the branch alex-issue17-suggestion

The issue we were having with the Table that the rows on page K were numbered starting at 1 rather than K*rows_per_page+1, appears to be fixed, and I see that there is a unit test for it.

Question: Are these cells-demo unit-tests running on every build (TravisCI), or are they only meant to be run locally for the time being?

I played with the filtration at the top of Col 1 and Col 2 but I didn't get any behavior that would match my intuition. Perhaps I don't have the needed context.

I did notice something odd when I played around with column sorting (with my modified version of table where each row has a number suffix and each column has a column prefix, in order to make each cell in the table unique. The unexpected behavior was that when I clicked the column to alter its sorting order (up or down arrow) it would take 4 and not 2 clicks to get back to the initial 0 to 99 ordering. For reference, the first row in these 4 cases are "hi 0", "hi 9", "hi 30", "hi 59"

Finally, I didn't find the capability of typing in a page number on the first column to just jump to that page, rather than clicking multiple times on the fwd or backward buttons, which @braxtonmckee described.

dkrasner commented 4 years ago

@atzannes cells-demo unit tests should be running in travis as of this PR

also I didn't find the column filter to do anything I would expect (on branch eric-issue-17-rebased), there is also a server side error which is thrown when you hit the sort arrows

 keymemo[row] = SortWrapper(r.sortsAs())
AttributeError: 'str' object has no attribute 'sortsAs'

which from I can see happens because there is an assumption that the renderFunction has a .sortAs() method, and the one in the rendererFun=lambda w, field: "hi" in the playground example does not

I am also not seeing a page input field (in addition to the arrows) - is this wanted?

@braxtonmckee @atzannes

braxtonmckee commented 4 years ago

So, it would be nice to know how many pages, be able to write into a text box. This functionality is supposed to be there but it's definitely broken.

Every cell is supposed to have a 'sortsAs()' which produces a value we can sort on. We are supposed to be converting everything to a cell - naked constants should get wrapped in a cell first. I think if things are not comparable, we should be sorting their type first.

the match filter should stringify and search for that substring. bonus points if we regex, since that would actually be useful.

Also, very important that when the table is re-building rows because data is coming in that we do not rebuild the entire table each time. It's bad because (a) it's slow and (b) if there is any UI state in flight it gets overwritten. When things are changing rapidly on the server it makes it (currently) impossible to filter or sort because the UX elements get replaced every update before you can hit them.

We should probably also put a (configurable) throttle on the number of updates per second on a given table, defaulting to, say, 10 / sec, to avoid swamping the browser.

dkrasner commented 4 years ago

@braxtonmckee I've been refactoring Table to fix some of the issues above, most importantly re-rendering, filtering, searching but need some clarification.

In looking at the MultiPageTable example in playground I made some assumptions about the Table cell API which would be good to confirm.

  1. The rendererFun takes two elements int and str and returns a Cell or anything that can be wrapped in a Cell, like a str. Is this right?

  2. What about the columnFun and rowFun's, do these return Cells or simply int/str values.

  3. What does the headerFun do?

  4. When we sort we want to sort the entire Table not just the current page? When we filter we want to filter the entire Table not just the current page? (if any of the above is a page-only operation then it can be handle client side)

braxtonmckee commented 4 years ago
  1. The render fun takes whatever object came out of the 'rowFun' to identify the row and whatever object came out of 'colFun' to identify the column and produces a cell (or something convertible to a cell). 'headerFun' takes a column ID and produces a cell.

  2. columnFun and rowFun can produce any python object that's value-like (a str, int, odb oject). these are used to identify the rows and columns.

  3. converts the output of a columnFun to a cell.

  4. yes, always on the whole set.