[GitHub] mistercrunch commented on issue #4756: Improve xAxis ticks, thinner bottom margin
mistercrunch commented on issue #4756: Improve xAxis ticks, thinner bottom margin URL: https://github.com/apache/incubator-superset/pull/4756#issuecomment-379033116 I'll wait on @graceguo-supercat's review before merging this This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] mistercrunch commented on issue #4756: Improve xAxis ticks, thinner bottom margin
mistercrunch commented on issue #4756: Improve xAxis ticks, thinner bottom margin URL: https://github.com/apache/incubator-superset/pull/4756#issuecomment-379032942 @williaster I agree with `flat` but didn't want to drive it myself as it may upset some users since we have to redefine/shorten `Adaptive formatting` as a prerequisite (I think the current definition was fine tuned by data scientists at Airbnb, forgot who but it's in the git history). It will also change the way thousands of line charts look like at Airbnb... You have my blessing to figure out better defaults that will please a majority of users. One key point is that day-of-week is very important to both Airbnb and Lyft given the strong weekly cycles, I wouldn't kick this out, better removing `` than removing 3-letters day-of-week. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] mistercrunch commented on issue #4756: Improve xAxis ticks, thinner bottom margin
mistercrunch commented on issue #4756: Improve xAxis ticks, thinner bottom margin URL: https://github.com/apache/incubator-superset/pull/4756#issuecomment-378828200 But yeah, while this doesn't fix everything, it addresses many visualizations defects and bugs with the xAxis and makes much better use of real estate. It also does not change the content of the labels which could be controversial and could be considered breaking backwards compatibility. It also improves the organization of axis-related controls for many chart types, making more consistent. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] mistercrunch commented on issue #4756: Improve xAxis ticks, thinner bottom margin
mistercrunch commented on issue #4756: Improve xAxis ticks, thinner bottom margin URL: https://github.com/apache/incubator-superset/pull/4756#issuecomment-378826578 Actually the default setting is `auto`, which turns into staggered for time series. We can change what `auto` means in the future. Same goes with `Adaptive formatting`. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] mistercrunch commented on issue #4756: Improve xAxis ticks, thinner bottom margin
mistercrunch commented on issue #4756: Improve xAxis ticks, thinner bottom margin URL: https://github.com/apache/incubator-superset/pull/4756#issuecomment-378815265 I'd also prefer `flat` to staggered as a default, but with the current default time formatter the labels overlap in some situation, depending on the time grain (some are wider) and on the width (d3.axis fits more or less tick density in ways we don't control). Maybe next PR on this we'd go: * new default `auto` format that's shorter than current `Adaptive formatting` * use `flat` as new default * somehow squeeze `Adaptive formatting` as the tooltip formatter (should be possible) The magic formatting is the part where the devil is in the detail and people may not agree on the exact layout. Here's a link to the code we have now: https://github.com/apache/incubator-superset/blob/master/superset/assets/javascripts/modules/dates.js#L15 Oh and we haven't even talked about locales :( This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] mistercrunch commented on issue #4756: Improve xAxis ticks, thinner bottom margin
mistercrunch commented on issue #4756: Improve xAxis ticks, thinner bottom margin URL: https://github.com/apache/incubator-superset/pull/4756#issuecomment-378777839 @graceguo-supercat @michellethomas would love for someone at Airbnb to sign off on this as it will affects a million line charts over there, but that should be for the best. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] mistercrunch commented on issue #4756: Improve xAxis ticks, thinner bottom margin
mistercrunch commented on issue #4756: Improve xAxis ticks, thinner bottom margin URL: https://github.com/apache/incubator-superset/pull/4756#issuecomment-378739069 I agree on the [un] manageability of this file. The challenge is that the different visualizations both from a controls and API standpoint have a lot in common. A lot of code is shared in odd intricate ways. It's a though puzzle. A balanced way to evolve this would be to refactor out portion of the code into smaller functions an unit test those. We've been talking about moving to `data-ui` / vx for a while, and maybe that's the end game and justify not spending too many cycles on `nvd3`. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services