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

Table per-cell line properties are a mess #2514

Closed
alexcjohnson opened this issue Mar 30, 2018 · 4 comments
Closed

Table per-cell line properties are a mess #2514

alexcjohnson opened this issue Mar 30, 2018 · 4 comments

Comments

@alexcjohnson
Copy link
Collaborator

Came up during #2511 - per-cell line properties behave oddly and it's difficult to figure out how to make them work well.

Here's the maximally weird case: per-cell line widths and semi-opaque borders (the weirdest is the cell with 5):
screen shot 2018-03-30 at 9 40 36 am

  • In order to fully display the borders where they overlap, we'd need to display all borders above all cell backgrounds - ie a separate border layer, with its own infinite scrollers etc... seems like more trouble (and overhead) than this edge case really warrants.
  • It's still ambiguous what order to draw the borders in - perhaps we want wider borders to override thinner ones, so we order them by thickness? That takes care of one common use case, using a thicker border to highlight a cell. But it doesn't cover using a different color to highlight a cell - actually I didn't explore array line colors at all when making this PR.
  • Another possibility I considered - both to fix Table borders sometimes get cut off #2401 and to clear up ambiguities - is to move the border to fully inside each cell. Then none of the borders would overlap, so the semi-transparent issue goes away, there's no ordering, and no clippath extension needed. But it messes up the most common case, of a consistent border everywhere, by doubling the effective width of interior lines vs peripheral lines.
  • Anyway to really do borders right we need a different API that respects the fact that there's one line per junction between cells, not one line per cell. I'm not sure what that would look like, but it would for example allow horizontal-only or vertical-only lines, or outlining a group of cells, neither of which we can do currently. Though THIS gets ambiguous with column reordering...
  • I almost want to remove per-cell lines entirely, and only support per-cell fill colors, which don't have ANY of these issues. We could then still add support for horizontal-only or vertical-only lines, and could support semiopaque borders without any of this weird overdrawing by only expanding the clippath along the outer edges. But as @etpinard points out this would be a breaking change so should wait for v2 wishlist for potential breaking changes since v1 #420 if we do it.

Thoughts @chriddyp @cldougl @VeraZab @cpsievert ?

@AbdealiLoKo
Copy link

AbdealiLoKo commented Mar 29, 2019

I had a bit of a naive question on the Table trace-type implementation.

It looks like what plotly is trying to do is:

  • Implement a Table in SVG
  • Creating a renderer using raw SVG shapes

This seems to be causing multiple minor issues like alignment issues in text, borders, etc.

Wouldn't it be easier to:

This way, plotly would get all alignment issues, colspan, auto-stretching, flexing, and so on solved (as HTML tables are battle tested) and can build more useful functionality on top of it.

@etpinard
Copy link
Contributor

@AbdealiJK you can review our discussion on the topic here: #991

@AbdealiLoKo
Copy link

I see. Thanks a lot for the reference. That clarifies.

@gvwilson
Copy link
Contributor

Hi - this issue has been sitting for a while, so as part of our effort to tidy up our public repositories I'm going to close it. If it's still a concern, we'd be grateful if you could open a new issue (with a short reproducible example if appropriate) so that we can add it to our stack. Cheers - @gvwilson

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants