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

[WIP] Table view #1834

Closed
wants to merge 282 commits into from
Closed

[WIP] Table view #1834

wants to merge 282 commits into from

Conversation

monfera
Copy link
Contributor

@monfera monfera commented Jun 29, 2017

As per discussion at #991

Most functions, attributes etc. are not working and not final.

Printing doesn't work as, apparently, nested font specs in an array (dimensions) balk when printing.

Test cases aren't added to this branch yet.

Shows a variety of column formats (font options, numeric options) and column rearrangement.

initial table

@monfera monfera force-pushed the table branch 2 times, most recently from 3452db9 to 8f0f78b Compare June 29, 2017 00:41
@monfera monfera self-assigned this Jun 29, 2017
@monfera
Copy link
Contributor Author

monfera commented Jun 29, 2017

Quick memo of our chat : the dimensions approach has some impedance mismatch with workspace so on a proposal by @etpinard we'll follow a heatmap "z" type approach.

To allow per cell styling, the attributes for left vs center vs right alignment, font styling, number formatting, background color etc. will admit either a singular value (e.g. the entire table uses the same font) or an array that follows the same nested structure eg.

align: [['left','right','center'], ['right','left','center'],['center','right','left']]

@etpinard specified these options for future consideration:

align: 'left' // applies to all cells
align: ['left', 'right', 'center'] // applies per row
align: [['left'],['right'], ['center']] applies to columns
align: [['left','right','center'], ['right','left','center'],['center','right','left']] // per-cell options

@monfera
Copy link
Contributor Author

monfera commented Jul 3, 2017

Feature list:
image

Above example shows these features:

  • cell styling: fill color, line color and line width
  • font styling: color, size, family (seen on 1st example atop)
  • cell and font styling (and most styling) is easy per row, per column or per cell, based on suggestion by @etpinard (makes mixing possible too)
  • text alignment: left-center-right (see per column), top-center-bottom (see first three columns)
  • settable row height (uniform) and column width (uniform or per column as seen)
  • cell padding
  • d3 style number formatting (%, $, thousands comma, decimal places, SI unit, filler char...)
  • prefix (e.g. 'US' before the '$')
  • suffix (eg. 'm' after 'k' in 1st column)
  • separate styling for the header row
  • drag-droppable columns
  • color scale bar shows up (not seen on above pic; its necessity is an open question)
  • image test mock
  • domain specification; layout margin awareness; title, general font, background color
  • event emission on column reordering (plotly_restyle)
  • printing now works

These need development for a MVP (IMO):

  • setting sizes in subplot proportions rather than pixels
  • better column drag&drop (to exceed Apple Numbers column drop)
  • truncating text if it's too long for cell
  • unifying column header styling with body cell styling
  • scrolling basics (mouse drag)
  • unit test cases
  • better image mocks

Todo before merging:

  • scrollbar
  • scroll with mousewheel, mouse gesture and keys

Todo before merging (from PR feedback items):

Add-on functions:

  • sorting by clicking on some little up/down arrow; multicolumn sorting?
  • column resizing on the UI (needed?)
  • sparklines
  • tooltip (useful if cell contents was truncated)

@monfera
Copy link
Contributor Author

monfera commented Jul 3, 2017

The mock that produces the above circus-themed cell smorgasbord:

Note: many attribs are inspired by @etpinard's suggestion so that an attrib can be

  • a scalar value that applies for the entire table
  • a vector that specifies the attrib per column
  • a vector of vectors; the deeper vector goes by the row

To add a row semantic, I simply reuse the last value, i.e. if there are 4 rows and an arbitrary number of columns, then this'll produce a red/blue/red/blue stripe for the 4 rows:

fill: {color: [["red"], ["blue"], ["red"], ["blue"]] }

Of course, the below version would produce per-column striping:
fill: {color: ["red", "blue", "red", "blue"] }

So, due to how it can be mixed and matched, and how the last array value is reused for remaining (unspecified) items, the attribute arrays can look quite heterogeneous. Also, attribute specs don't have to match one another's structure in the JSON.

{
    "layout": {
        "width": 1184,
        "height": 400,
        "title": "Widget parameters and cost",
        "font": {
            "color": "blue",
            "size": 2,
            "family": "sans-serif"
        },
        "margin": {"t": 106, "r": 60, "b": 54, "l": 60}
    },

    "data": [{

        "type": "table",

        "domain": {
            "x": [0.05, 0.95],
            "y": [0.05, 0.85]
        },

        "width": [110, 60, 90, 90, 80, 70, 80, 60, 100, 70],
        "height": 30,

        "align": ["right", "center", "center", "right", "right", "right", "left", "right"],
        "valign": ["top", "bottom", "center"],

        "prefix": ["", "", "", "", "US ", ""],
        "suffix": ["m", ""],

        "font": {
            "color": [
                ["rgb(214,235,14)", "rgb(24,251,11)", "rgb(109,103,67)", "rgb(152,192,23)", "rgb(80,207,160)", "rgb(84,134,44)", "rgb(222,105,214)", "rgb(76,181,63)", "rgb(110,173,82)", "rgb(144,56,245)", "rgb(48,196,142)", "rgb(190,102,179)", "rgb(115,83,61)", "rgb(54,231,74)", "rgb(235,105,39)", "rgb(1,128,246)", "rgb(205,58,176)", "rgb(34,128,113)", "rgb(222,81,3)", "rgb(18,252,69)"],
                ["rgb(107,128,223)", "rgb(95,62,207)", "rgb(110,65,159)", "rgb(17,249,0)", "rgb(184,189,240)", "rgb(234,55,166)", "rgb(254,75,119)", "rgb(215,70,114)", "rgb(102,160,80)", "rgb(7,190,174)", "rgb(177,25,224)", "rgb(237,181,164)", "rgb(122,240,243)", "rgb(54,245,219)", "rgb(65,30,43)", "rgb(67,161,153)", "rgb(77,77,169)", "rgb(11,144,192)", "rgb(27,193,182)", "rgb(176,134,145)"],
                ["rgb(214,204,7)", "rgb(228,251,163)", "rgb(22,8,80)", "rgb(129,125,180)", "rgb(229,211,97)", "rgb(14,77,169)", "rgb(31,68,23)", "rgb(219,106,81)", "rgb(113,123,241)", "rgb(113,228,139)", "rgb(115,124,86)", "rgb(185,227,179)", "rgb(126,218,33)", "rgb(83,39,227)", "rgb(43,40,158)", "rgb(244,77,12)", "rgb(19,15,168)", "rgb(212,66,245)", "rgb(175,76,103)", "rgb(250,61,197)"],
                "red",
                ["magenta", "blue"],
                ["rgb(47,249,84)", "rgb(186,219,118)", "rgb(116,107,204)", "rgb(255,6,79)", "rgb(157,34,21)", "rgb(90,61,107)", "rgb(12,195,215)", "rgb(16,22,219)", "rgb(111,207,137)", "rgb(106,15,130)", "rgb(156,131,30)", "rgb(178,28,31)", "rgb(90,192,19)", "rgb(188,146,244)", "rgb(83,1,104)", "rgb(74,206,15)", "rgb(200,23,125)", "rgb(94,68,55)", "rgb(25,43,18)", "rgb(72,79,254)"]
            ],
            "size": 12,
            "family": "sans-serif"
        },

        "line": {
            "color": [
                ["grey"]
            ],
            "width": 0.5
        },

        "fill": {
            "color": [
                ["#e0f3f8", "#abd9e9","#e0f3f8", "#abd9e9","#e0f3f8", "#abd9e9","#e0f3f8", "#abd9e9","#e0f3f8", "#abd9e9","#e0f3f8", "#abd9e9","#e0f3f8", "#abd9e9"]
            ]
        },

        "labelfont": {
            "family": "PT Sans Narrow",
            "size": 16,
            "weight": "bold",
            "color": "grey"
        },
        "values": [
            [32000, 162666, 32000, 162666, 32000, 162666, 32000, 162666, 32000, 162666, 32000, 162666, 32000, 162666, 32000, 162666, 32000, 162666, 32000, 162666],
            [268630, 489543, 379086, 600000, 489543, 268630, 600000, 379086, 268630, 489543, 379086, 600000, 489543, 268630, 600000, 379086, 268630, 489543, 379086, 600000],
            ["A", "A", "B", "AB", "Y", "Z", "Z", "A", "B", "B", "Y", "Y", "Z", "A", "A", "B", "Y", "Y", "Z", "Z"],
            [160, 1324, 252, 1711, 173, 624, 228, 2474, 371, 3103, 312, 2408, 460, 1727, 754, 2457, 227, 1963, 354, 2797],
            [1125, 9274, 2735, 16920, 2193, 20704, 4814, 34689, 5565, 46554, 5225, 38887, 9746, 37715, 17798, 55250, 5697, 49083, 11226, 81939],
            [9, 794, 34, 1409, 17, 536, 39, 7133, 115, 9843, 107, 7017, 273, 5131, 1210, 9032, 82, 7312, 252, 16778],
            [0.2366, 0.3269, 0.3471, 0.4373, 0.4575, 0.106, 0.568, 0.2164, 0.2366, 0.3269, 0.3471, 0.4373, 0.4575, 0.106, 0.568, 0.2164, 0.2366, 0.3269, 0.3471, 0.4373],
            [5137, 113712, -5909, 102666, -16955, 135803, -28000, 124758, 5137, 113712, -5909, 102666, -16955, 135803, -28000, 124758, 5137, 113712, -5909, 102666],
            [98453, 319366, 234498, 455411, 391333, 147711, 479081, 208910, 124041, 344954, 280876, 501789, 368624, 98453, 429823, 234498, 170419, 391333, 258167, 479081],
            [1417, 23377, 3376, 33336, 5635, 10812, 6898, 21273, 2484, 35126, 5626, 51096, 7384, 22020, 18912, 52449, 7498, 87528, 11359, 107154]
        ],
        "labels": [
            "Height",
            "Width",
            "Cylinder",
            "Weight",
            "Cost",
            "Penalty",
            "H/W",
            "Min H/W",
            "Diameter",
            "RF block"
        ],
        "valueformat": [
            ".2s",
            "#0x",
            null,
            ",.2f",
            "$,",
            "_>5",
            ",.1%",
            null,
            null,
            null
        ]
    }]
}

@monfera
Copy link
Contributor Author

monfera commented Jul 3, 2017

I'm leaning toward deleting the colorbar. The original idea was that, if the user chooses to color cells by some value, then the colorbar would be handy. But the issue is that there's a combinatorial explosion of possible wishes, eg. the user would use coloring for one column, or multiple columns of the same unit / measure / etc. (comparables); but perhaps it would be rows, or a rectangular area in the table. Heterogeneous coloring can't be done because a colorbar supports one scale only.

Yet it's simple to achieve the desired effect, because as with the other cell styling options, a per-row, per-column or per-cell color can be directly specified.

If the decision is to retain the colorbar, I could use guidance on how exactly it should behave and how it should interact with possible cell coloring specification that might be in conflict.

@jackparmer
Copy link
Contributor

I'm leaning toward deleting the colorbar.

Yes, definitely. The Python table figure factory doesn't have a colorbar either.

@jackparmer
Copy link
Contributor

jackparmer commented Jul 3, 2017

I agree with the ✅ 's in the MVP section. It seems like those will get us to parity with the Python table figure factory, which we already know is a success.

We can let future features be driven by user and customer demand.

scrolling or pagination (or and?)

Offering one or the other seems more than sufficient. FWIW, I've seen table pagination with DataTables a fair amount in Shiny apps:
https://datatables.net/examples/basic_init/alt_pagination.html

@monfera
Copy link
Contributor Author

monfera commented Jul 5, 2017

@etpinard @alexcjohnson have you got a preference for styling the header row in the JSON spec? Some options:

  1. There is no concept of a header row; users need to use the 1st row (values and styling) as a header row, with whatever formatting they need (formatting can be per row, column and cell for the table)
  2. There is a separate spec in the JSON for the header row; its substructure is the same as the substructure of the table (though 2D arrays won't make sense if we stick to single-row headers), i.e. the values/style spec is duplicated like:
{ 
  "width: [50, 30, ...], // width has to be shared so it's at the top level
  "header": {
    "values": ["Horse power", "Volume", ...],
    "font": {"color": ["red", "blue", ...]},
    "fill": {...}
    ...
   },
  "cells": {
    "values": [[110,202,52,...], [23,45,9,...], ...],
    "font": {"color": ["black", "grey", ...]},
    "fill": {...}
   },
  ...
}

@alexcjohnson
Copy link
Collaborator

Lets go with (2) - separate spec for the header row, and yes, single-row headers. This is a very common use case so shouldn't require turning all the styles into arrays. Also fits in with the analogous heatmap case - in which axis labels are specified independently from the data.

@etpinard
Copy link
Contributor

etpinard commented Jul 5, 2017

Yeah (2) sounds good. I'm not a fan of the root-level width attribute name as it might be confused for layout.width, but I can't think of a better name and annotations already have a width attribute, so 👍

@monfera
Copy link
Contributor Author

monfera commented Jul 5, 2017

Thanks for the answers! Yes some attribs e.g. align were taken from annotations.

@monfera
Copy link
Contributor Author

monfera commented Jul 5, 2017

columnwidth or {columns: {width: }} would work if you prefer that

@alexcjohnson
Copy link
Collaborator

I like columnWidth, good idea! Makes it clear that it's not the overall table width, and can be an array.

@etpinard
Copy link
Contributor

etpinard commented Jul 5, 2017

I like columnWidth

columwidth please in this case. plotly.js doesn't like 🐫 cases in attributes.

@monfera
Copy link
Contributor Author

monfera commented Jul 6, 2017

Non-garish table example:

image

@alexcjohnson
Copy link
Collaborator

heterogeneous row heights (needed if text is wrapped); maybe option for keeping rows of homogeneous height? e.g. only one row needs to increase its height to fit multiple rows; should the other rows increase in height?

I don't think we need an option for this (if I'm understanding you right) - this should work just like regular html tables, ie all rows get the same height, which is enough to fit the tallest contents. Otherwise items in a row lose connection to each other.

breaking text to multiple rows at spaces; breaking too long words nevertheless if space based breaking is not enough; perhaps truncation (with or without an ellipsis) as a boolean option?

links (if needed) and perhaps other content types?

Lets use svgTextUtils.convertToTspans so we get all the pseudo-html features. And perhaps we can work wrapping into that routine so it can be used elsewhere too? 👼 Truncation as an option is a good idea, but I'd think for tables wrapping should be the default.

["Min H/W"],
["Diameter"],
["RF block"]
],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does header.values need to be 2D, or is 1D sufficient?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, it handles a 1D array just fine, I'll need to update the mock, thanks for the reminder!

@monfera
Copy link
Contributor Author

monfera commented Jul 6, 2017

@alexcjohnson thanks for your feedback, I'll look into the text wrapping! btw. when I wrote 'heterogeneous row heights' I referred to the possibility that some rows (the entire row ofc.) would be taller than others to wrap long contents into multiple lines - yes an alternative is to make all rows of equal height (the max). Given that we'll have pagination, and the table may have a lot of rows, probably heterogeneous row heights would provide a better user experience as a single outlier long line would make the entire table look weird, pls. let me know if you see otherwise. (I wasn't shooting for broken lines ie. the cells of a specific line would be of equal height, as per the max wrap count in the row).

An aside is that HTML tables are rather sophisticated for text wrapping and I'm not currently shooting for feature parity with it. For example, if left unspecified, HTML tables will adjust the table column widths so as to maximize the screen use. For example, a column with more / longer content would end up being wider. I doubt the HTML table does an actual MILP optimization but at the simplest, it may use some heuristic eg. total text length, which would of course be crude, as non-proportional fonts and non-uniformly breakable text (dependent on word length, word count etc.) make it more complex than if text were like sand or water 🚰

@alexcjohnson
Copy link
Collaborator

Given that we'll have pagination, and the table may have a lot of rows, probably heterogeneous row heights would provide a better user experience as a single outlier long line would make the entire table look weird, pls. let me know if you see otherwise. (I wasn't shooting for broken lines ie. the cells of a specific line would be of equal height, as per the max wrap count in the row).

Ah great, I thought I must have misunderstood you, thanks for clarifying! Yes, I think you're correct that each row should size to its own contents, not the tallest of all contents. I wouldn't bother making that an option, at least not initially. Raises a question about how pagination should work with these variable-length rows, do you go for constant number of rows or (as close as possible) constant total height? If the latter, there will still be a possibility that a single row is bigger than the allotted size. To me that kind of consideration kind of argues for scrolling instead of pagination - and personally I think I kind of prefer scrolling anyway, but it's probably a good deal harder to implement in a performant way.

An aside is that HTML tables are rather sophisticated for text wrapping and I'm not currently shooting for feature parity with it.

Way too sophisticated, I've always been confused by the decisions HTML layout engines make there. Simple and explicit is way better for our purposes.

@monfera
Copy link
Contributor Author

monfera commented Jul 6, 2017

@alexcjohnson thinking about the latter, because otherwise the page height will be overly variable, and the pagination buttons must be kept at place, i.e. there would be too much variance in the gap above the pagination buttons, and a large gap sends the misleading message: I'm the last page!

As the user can jump to arbitrary pages, the entire table has to be wrapped upfront... so we're kinda reimplementing the HTML table functionality... scrolling is the same in this regard (all wrapping has to be precomputed). Yet I'm partial for pagination, at least in the 1st version, for some reasons (just a sampler):

  • limited, bounded DOM element count; the mere element count is a big factor in interaction performance, even if we don't translate stuff after the initial render
  • not straying from plotly.js patterns (I don't think we have scrolling widgets)
  • column reordering is much simpler (the header would need to stay in place while normal rows would scroll past)

So as you can see, wrapping, while it sounds innocent as a feature request, it triggers a whole lot of design and implementation choices and we'll learn of most of them during the work :-)

@alexcjohnson
Copy link
Collaborator

I agree that pagination will likely be easier to implement - and I'm not particularly strongly opinionated about which we go with - but to your points:

page height will be overly variable, and the pagination buttons must be kept at place, i.e. there would be too much variance in the gap above the pagination buttons, and a large gap sends the misleading message: I'm the last page!

This will be an issue - at least in edge cases - no matter what for a paginated table. But scrolling tables can just show partial rows and fill whatever space they are allotted (unless the whole table is smaller than the given space, which is also fine).

limited, bounded DOM element count; the mere element count is a big factor in interaction performance, even if we don't translate stuff after the initial render

In principle I would imagine a container big enough for all the rows, but then only rendering the visible ones... so then the element count would be only marginally larger for scrolling than for pagination, though the frequency of changing which ones are rendered would be much larger.

not straying from plotly.js patterns (I don't think we have scrolling widgets)

We do have scrolling legends and dropdowns...

column reordering is much simpler (the header would need to stay in place while normal rows would scroll past)

Not sure I follow... I guess with pagination we could make each column (header included) a group, and move them together, whereas with scrolling it would be easier to make the headers inside one container and the data inside another container, so there would be two things to move in sync whenever a column moves? So pagination does seem simpler but I wouldn't characterize it as much simpler.

wrapping, while it sounds innocent as a feature request

Yah for sure, all sorts of issues around wrapping. Just to be clear though, it's not just automatic wrapping that triggers this, but also <br> in the text if we support pseudo-html, and also font size if that's allowed to be variable.

@monfera
Copy link
Contributor Author

monfera commented Jul 6, 2017

btw. if scrolling were needed, we'd probably need to do an infinite revolver to keep element bounded, e.g. https://bl.ocks.org/monfera/8b87a9aade2214fc15ea76bdf7abe11a

We'd need to do something similar because in my experience a long SVG surface (multiple k pixels tall or wide) will be slow even if only the visible part is rendered, so basically we'd just mimic the scrolling like in the above example.

Thanks for the clarifications on the other points! Brw if we want to initially go with a scroller rather than a paginator pls tell me today so I can pivot early tomorrow my time.

@alexcjohnson
Copy link
Collaborator

Ah, the infinite revolver is a cool idea, with the two panes trading order!

Dunno if @jackparmer or @etpinard have other opinions, but from my perspective scrolling is worth it if:

  • It can work just like we have it for legends, for example the legend_scroll mock: wheel scroll with the same inertia as regular page scrolls, and a permanently visible drag handle (though I notice the legend scroll handle doesn't have a size proportional to the visible fraction, we should fix that).
  • It's not going to add more than a couple of days work vs. pagination

@monfera
Copy link
Contributor Author

monfera commented Jul 7, 2017

I'll do a morning sprint on a crude scroller just to see how it behaves performance and jankiness wise.

@etpinard
Copy link
Contributor

etpinard commented Oct 5, 2017

now done in #2052

@etpinard etpinard closed this Oct 5, 2017
@etpinard etpinard deleted the table branch October 5, 2017 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants