[GitHub] mistercrunch commented on issue #4756: Improve xAxis ticks, thinner bottom margin

2018-04-05 Thread GitBox
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

2018-04-05 Thread GitBox
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

2018-04-04 Thread GitBox
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

2018-04-04 Thread GitBox
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

2018-04-04 Thread GitBox
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

2018-04-04 Thread GitBox
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

2018-04-04 Thread GitBox
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