Replies: 1 comment
-
On the topic of " DataTable, ViewPort, ViewPortColumns, RowData and Columns" To build on top of Junaid's suggestion on defining the |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
Currently there is concept of ViewPort which is designed to hold definition of what the user view per session and can have a different list of columns from DataTable
Usages of these concepts where VUU is getting the data to send to front end is not always respecting this separation of concerns and require some work to support this.
Below is some suggestion from Junaid on how to approach this:
1) Table meta sent to the UI should be based on ViewPortDef instead of TableDef
Currently, we have a concept of
TableDef.columns
andViewPortDef.columns
, but we're not using the latter anywhere. Although, it seems like the initial intention was to decouple a table (server-side data store) from its viewport (what is shown to the user). If that's what we want, then we should sendViewPortDef.columns
to the UI inTableMeta
RPC response rather than the columns fromTableDef
. That way we can easily hide server only columns from the UI, and can add extra derived/mapped columns to the ViewPort that can be calculated on the fly without having to store their values on the server (kind of similar to how calculated columns work).2) Decouple APIs of DataTable, ViewPort, ViewPortColumns, RowData and Columns (can make you run in circles)
Let's start with different ways we can use to build variations of the same underlying row data:
dataTable.pullRow(key): RowData
returns you the actual row stored in the tabledataTable.pullRow(key, vpColumns): RowData
returns you the row based on columns passed (source of confusion, since both this and the above have the same name but might contain different fields)dataTable.tableDef.columns.map(c => c.name -> c.getData(rowData)).toMap
(should be exactly the same as the first)dataTable.pullRowAsArray(key, vpColumns): Array[Any]
inMemDataTable.viewPortColumns.getCloumns().map(...).toMap
viewPortColumns.pullRow(key, rowData): RowData
(why do we need this? Why not simply use thedataTable.pullRow(key, vpVolumns)
?)viewPortColumns.pullRowAlwaysFilter(key, rowData): RowData
(Similarly, why do we need to have two different ways to do the same thing?)viewPortColumns.getColumns().map(c => c.name -> c.getData(rowData)).toMap
(the columns should not even be exposed in this API, tbh)rowData.pullRowAsArray(vpColumns): Array[Any]
(probably array is only needed when sending data to the UI, why do we need a similar method on the table? I can get row from the table and apply this method to get exactly the same array.)As you can see, there's too much duplication, and it's quite confusing. For now it only works because
vpColumns
are built from the table def, which means they are exactly the same as table def columns except for any extraCalculatedColumns
. And therefore, you can always build table row from view port row and vice versa. Which ideally, should only be a one way relationship, you should only be able to build view port row from table row, not the other way round because that should never be needed (you can always pull table row from a table, why would you want to build the table row from a view port row?)Potential solution:
I think we should have a way to differentiate between table row and view port row, maybe by introducing subtypes of
RowData
:TableRowData
andViewPortRowData
. Common methods can live onRowData
and specific methods i.e.pullRowAsArray
should only exist where actually needed i.e.ViewPortRowData
. Also, there can be a function onTableRowData
to convert it toViewPortRowData
, as this should always be possible. View port row is derived from table row.Once we have this distinction, we can remove all extra
pull
methods onDataTable
except fordataTable.pullRow(key): TableRowData
. This way data table would know nothing ofViewPortColumns
object. Then to get view port row from table we can simply dodataTable.pullRow(key).toViewPortRow(vpColumns)
. Can also remove all the.pullRow*
methods fromViewPortColumns
.3) Typeahead should have viewport def / columns in context
One last thing required would be to use viewport columns for typeahead as well, because filters on the UI are applied to the viewport columns and not to the underlying table columns.
Beta Was this translation helpful? Give feedback.
All reactions