src-d / sourced-ui

source{d} UI
https://sourced.tech
Apache License 2.0
6 stars 15 forks source link

On chart edit page a chart isn't visible fully #113

Open smacker opened 5 years ago

smacker commented 5 years ago
Screenshot 2019-06-17 at 12 15 43

Regression after the redesign. It has lables on x axis. But I can't see them here. Only on the dashboard.

smacker commented 5 years ago

Actually it is REALLY BAD. I was trying to fix some other charts and I can't because I don't see changes and can't understand what the chart is showing. It would prevent creating new charts as well.

marnovo commented 5 years ago

Same comment as in https://github.com/src-d/sourced-ui/issues/114#issuecomment-503971364:

@smacker @ricardobaeta isn't this caused (or fixed) by some layout or overflow property?

smacker commented 5 years ago

I would suggest making Long title s... instead of displaying it fully and breaking the layout. To do that, there should be some overflow property for sure.

ricardobaeta commented 5 years ago

@smacker Thanks!

dpordomingo commented 5 years ago

As far as I could test it, this problem is already happening in current master, so I'd say it's related to superset itself, istead of to typography issues, so imo it should not block typography development.

The problem appears when the chart title is long enough that it does not fit in the browser window, no matter if we use branding-typography branch or master. I managed to find one example of a chart that we already have in master, that already breaks the chart as described in this issue.

Imo it's a good idea to fix the bug, but due to our current time limit I'd purpose to do a "patch" or workaround, that will work in "all" circumstances:

As a v2 solution could be to use breakpoints to avoid edge cases as described above. As a v3 solution could be to rework chart markup/css to be fully fluid.

imo v2 or v3 should be done in the next iteration, and release the beta with the quick fix.

example:
max-width: 358px;
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;

You'll find below two examples of how our current charts are affected by this issue already in master

:point_down: Enough window size: image

:point_down: Smaller window size: image


And same data with a different chart layout.

:point_down: Enough window size: image

:point_down: Smaller window size: image

marnovo commented 5 years ago

@dpordomingo, I was seeing this now with @ricardobaeta and propose using this option:

define a max-width + ellipsis for the chart title

But with a twist: We since it's about defining an arbitrary width, we can do this using calc() like:

calc(total width of the container, e.g. 100% - width of the maximum size of the top right toolbar, e.g. xx px or whatever metric we use)

This way the ellipsed element width can vary based on the window size, reducing wasted whitespace (since the toolbar width varies less than the overall container) and making it much less likely to break at small widths.

Example: https://www.w3schools.com/code/tryit.asp?filename=G595AP15C6RU