Add support for axis.x.tick.multilineMax #2364

jcsmorais · 2018-05-10T22:02:56Z

Summary:
If you have long category names you can use axis.x.tick.multiline to make these names render properly into multiple lines. A problem arises when category names have variable lengths and your chart has a fixed height, you'll then start seeing these names being cut off.

On this PR I'm adding support for a way to define the maximum amount of lines supported on a given chart through a new optional config flag: axis.x.tick.multilineMax.

Documentation:

Examples:

https://codepen.io/jcsmorais/pen/KRorNQ

https://codepen.io/jcsmorais/pen/demQgM

Related issues:

Notes:

~~Struggled a bit with adding tests and figuring out how these are organized, figured I'd get feedback first and I'll try to add these later.~~ Edit: kept struggling to add test cases mostly due to the nested describes that rely on data set by their parents, which imho doesn't make it easy to understand what's going on, nor what what the data dependencies are, plus it makes it hard to run specific test cases in isolation. Maybe that's something that would be worth investing time on? Either way, I was able to add a test case to confirm the expected behavior.
~~Wasn't able to add documentation, kept getting some errors while running npm run build:docs.~~ Edit: documentation added for both x.axis.tick.multiline and x.axis.tick.multilineMax.

codecov-io · 2018-05-10T22:12:01Z

Codecov Report

Merging #2364 into master will increase coverage by 0.07%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2364      +/-   ##
==========================================
+ Coverage   77.15%   77.23%   +0.07%     
==========================================
  Files          51       51              
  Lines        4124     4138      +14     
==========================================
+ Hits         3182     3196      +14     
  Misses        942      942

Impacted Files	Coverage Δ
src/config.js	`95.83% <ø> (ø)`	⬆️
src/axis.js	`96.08% <100%> (+0.11%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87dc577...0f9f840. Read the comment docs.

jcsmorais · 2018-05-12T07:08:15Z

@kt3k @masayuki0812 @Aendrew any chance you guys could review this PR?
Thanks

kt3k · 2018-05-15T15:01:57Z

src/config.js

@@ -107,6 +107,7 @@ c3_chart_internal_fn.getDefaultConfig = function () {
        axis_x_tick_rotate: 0,
        axis_x_tick_outer: true,
        axis_x_tick_multiline: true,
+        axis_x_tick_multiline_max: 0,


I think this should be axis_x_tick_multilineMax because axis_x_tick_multiline is already boolean and settings axis_x_tick_multiline_max number makes the type of axis_x_tick_multiline ambiguous.

(so the test case may look like:

args.axis.x.tick = { multiline: true, multilineMax: 2, }

Applied suggested changes.

kt3k · 2018-05-15T15:05:20Z

src/axis.js

+    for (var i = max-1 ; i >= 0 ; i--) {
+        var available = ellipsified[i].length;
+
+        ellipsified[i] = ellipsified[i].substr(0, available-remaining).padEnd(available, '.');


padEnd doesn't seem available on IE11 which c3 still supports. Probably we need util function for this.

Wasn't sure if IE11 was one of C3's supported browsers.
I'm adding String.padEnd as a polyfill then.

kt3k · 2018-05-15T15:22:11Z

Thanks for the contribution! The suggestion itself seems good to me! I commented on some points.

kept getting some errors while running npm run build:docs

build:docs task requires ruby and bundler gem installed. Please see this document.

jcsmorais · 2018-05-15T17:59:47Z

@kt3k thanks for your review, in the meanwhile I already applied all suggested changes.
Can you take a 2nd look please?

I also gave it a 2nd shot at generating docs, unfortunately without success.
Maybe that's something that can be addressed separately in another PR?
I also noticed that currently there's no documentation for axis.x.tick.multiline either, so maybe we can bundle these two together?

Not sure if there's something misconfigured on my machine but here are the errors I'm getting:

npm run watch:docs

> c3@0.6.0 watch:docs /Users/joao/Projects/c3
> bundle exec middleman

Expected string default value for '--instrument'; got false (boolean)
Expected string default value for '--reload-paths'; got false (boolean)
Expected string default value for '--instrument'; got false (boolean)
== The Middleman is loading
/usr/local/lib/ruby/gems/2.5.0/gems/builder-3.0.4/lib/builder/xchar.rb:111: warning: constant ::Fixnum is deprecated
== The Middleman is standing watch at http://0.0.0.0:4567
== Inspect your site configuration at http://0.0.0.0:4567/__middleman/

When I try to access the documentation on http://0.0.0.4567/ I get the following errors:

Haml::TempleEngine: Option :ugly is invalid
Haml::TempleEngine: Option :outvar is invalid
/usr/local/lib/ruby/gems/2.5.0/gems/tilt-1.4.1/lib/tilt/haml.rb:55: warning: Haml::Engine#precompiled_method_return_value at /usr/local/Cellar/ruby/2.5.1/lib/ruby/2.5.0/forwardable.rb:157 forwarding to private method Haml::TempleEngine#precompiled_method_return_value
Haml::TempleEngine: Option :locals is invalid
Haml::TempleEngine: Option :ugly is invalid
Haml::TempleEngine: Option :outvar is invalid
/usr/local/lib/ruby/gems/2.5.0/gems/tilt-1.4.1/lib/tilt/haml.rb:55: warning: Haml::Engine#precompiled_method_return_value at /usr/local/Cellar/ruby/2.5.1/lib/ruby/2.5.0/forwardable.rb:157 forwarding to private method Haml::TempleEngine#precompiled_method_return_value
Haml::TempleEngine: Option :locals is invalid
Haml::TempleEngine: Option :ugly is invalid
Haml::TempleEngine: Option :outvar is invalid
/usr/local/lib/ruby/gems/2.5.0/gems/tilt-1.4.1/lib/tilt/haml.rb:55: warning: Haml::Engine#precompiled_method_return_value at /usr/local/Cellar/ruby/2.5.1/lib/ruby/2.5.0/forwardable.rb:157 forwarding to private method Haml::TempleEngine#precompiled_method_return_value
NameError: uninitialized constant #<Class:0x00007fae117d7528>::EXPR_BEGEXPR_BEGEXPR_BEGEXPR_BEGEXPR_CMDARGEXPR_DOTEXPR_ARGEXPR_BEG
	/Users/joao/Projects/c3/docs/_reference_item_link.haml:1:in `block in singleton class'
	/Users/joao/Projects/c3/docs/_reference_item_link.haml:-8:in `instance_eval'
	/Users/joao/Projects/c3/docs/_reference_item_link.haml:-8:in `singleton class'
	/Users/joao/Projects/c3/docs/_reference_item_link.haml:-10:in `__tilt_70192797114840'
	/usr/local/lib/ruby/gems/2.5.0/gems/tilt-1.4.1/lib/tilt/template.rb:170:in `call'
	/usr/local/lib/ruby/gems/2.5.0/gems/tilt-1.4.1/lib/tilt/template.rb:170:in `evaluate'
	/usr/local/lib/ruby/gems/2.5.0/gems/tilt-1.4.1/lib/tilt/haml.rb:24:in `evaluate'
	/usr/local/lib/ruby/gems/2.5.0/gems/tilt-1.4.1/lib/tilt/template.rb:103:in `render'
	/usr/local/lib/ruby/gems/2.5.0/gems/middleman-core-3.2.2/lib/middleman-core/core_extensions/rendering.rb:280:in `render_individual_file'
	/usr/local/lib/ruby/gems/2.5.0/gems/middleman-core-3.2.2/lib/middleman-core/core_extensions/rendering.rb:225:in `render'
	/usr/local/lib/ruby/gems/2.5.0/gems/middleman-core-3.2.2/lib/vendored-middleman-deps/padrino-helpers-0.11.4/lib/padrino-helpers/render_helpers.rb:53:in `partial'
	/Users/joao/Projects/c3/docs/_reference_menu_item.haml:5:in `block in singleton class'
	/Users/joao/Projects/c3/docs/_reference_menu_item.haml:-8:in `instance_eval'
	/Users/joao/Projects/c3/docs/_reference_menu_item.haml:-8:in `singleton class'
	/Users/joao/Projects/c3/docs/_reference_menu_item.haml:-10:in `__tilt_70192797114840'
	/usr/local/lib/ruby/gems/2.5.0/gems/tilt-1.4.1/lib/tilt/template.rb:170:in `call'
	/usr/local/lib/ruby/gems/2.5.0/gems/tilt-1.4.1/lib/tilt/template.rb:170:in `evaluate'
	/usr/local/lib/ruby/gems/2.5.0/gems/tilt-1.4.1/lib/tilt/haml.rb:24:in `evaluate'
	/usr/local/lib/ruby/gems/2.5.0/gems/tilt-1.4.1/lib/tilt/template.rb:103:in `render'
	/usr/local/lib/ruby/gems/2.5.0/gems/middleman-core-3.2.2/lib/middleman-core/core_extensions/rendering.rb:280:in `render_individual_file'
	/usr/local/lib/ruby/gems/2.5.0/gems/middleman-core-3.2.2/lib/middleman-core/core_extensions/rendering.rb:225:in `render'
	/usr/local/lib/ruby/gems/2.5.0/gems/middleman-core-3.2.2/lib/vendored-middleman-deps/padrino-helpers-0.11.4/lib/padrino-helpers/render_helpers.rb:53:in `partial'
	/Users/joao/Projects/c3/docs/reference.html.haml:10:in `block in singleton class'
	/Users/joao/Projects/c3/docs/reference.html.haml:-8:in `instance_eval'
	/Users/joao/Projects/c3/docs/reference.html.haml:-8:in `singleton class'
	/Users/joao/Projects/c3/docs/reference.html.haml:-10:in `__tilt_70192797114840'
	/usr/local/lib/ruby/gems/2.5.0/gems/tilt-1.4.1/lib/tilt/template.rb:170:in `call'
	/usr/local/lib/ruby/gems/2.5.0/gems/tilt-1.4.1/lib/tilt/template.rb:170:in `evaluate'
	/usr/local/lib/ruby/gems/2.5.0/gems/tilt-1.4.1/lib/tilt/haml.rb:24:in `evaluate'
	/usr/local/lib/ruby/gems/2.5.0/gems/tilt-1.4.1/lib/tilt/template.rb:103:in `render'
	/usr/local/lib/ruby/gems/2.5.0/gems/middleman-core-3.2.2/lib/middleman-core/core_extensions/rendering.rb:280:in `render_individual_file'
	/usr/local/lib/ruby/gems/2.5.0/gems/middleman-core-3.2.2/lib/middleman-core/core_extensions/rendering.rb:157:in `render_template'
	/usr/local/lib/ruby/gems/2.5.0/gems/middleman-core-3.2.2/lib/middleman-core/sitemap/resource.rb:143:in `block in render'
	/usr/local/lib/ruby/gems/2.5.0/gems/activesupport-3.2.22.5/lib/active_support/notifications.rb:125:in `instrument'
	/usr/local/lib/ruby/gems/2.5.0/gems/middleman-core-3.2.2/lib/middleman-core/util.rb:42:in `instrument'
	/usr/local/lib/ruby/gems/2.5.0/gems/middleman-core-3.2.2/lib/middleman-core/application.rb:215:in `instrument'
	/usr/local/lib/ruby/gems/2.5.0/gems/middleman-core-3.2.2/lib/middleman-core/sitemap/resource.rb:16:in `instrument'
	/usr/local/lib/ruby/gems/2.5.0/gems/middleman-core-3.2.2/lib/middleman-core/sitemap/resource.rb:114:in `render'
	/usr/local/lib/ruby/gems/2.5.0/gems/middleman-core-3.2.2/lib/middleman-core/core_extensions/request.rb:256:in `process_request'
	/usr/local/lib/ruby/gems/2.5.0/gems/middleman-core-3.2.2/lib/middleman-core/core_extensions/request.rb:206:in `block in call!'
	/usr/local/lib/ruby/gems/2.5.0/gems/middleman-core-3.2.2/lib/middleman-core/core_extensions/request.rb:205:in `catch'
	/usr/local/lib/ruby/gems/2.5.0/gems/middleman-core-3.2.2/lib/middleman-core/core_extensions/request.rb:205:in `call!'
	/usr/local/lib/ruby/gems/2.5.0/gems/middleman-core-3.2.2/lib/middleman-core/core_extensions/request.rb:191:in `call'
	/usr/local/lib/ruby/gems/2.5.0/gems/rack-1.6.8/lib/rack/urlmap.rb:66:in `block in call'
	/usr/local/lib/ruby/gems/2.5.0/gems/rack-1.6.8/lib/rack/urlmap.rb:50:in `each'
	/usr/local/lib/ruby/gems/2.5.0/gems/rack-1.6.8/lib/rack/urlmap.rb:50:in `call'
	/usr/local/lib/ruby/gems/2.5.0/gems/rack-1.6.8/lib/rack/showexceptions.rb:24:in `call'
	/usr/local/lib/ruby/gems/2.5.0/gems/rack-1.6.8/lib/rack/head.rb:13:in `call'
	/usr/local/lib/ruby/gems/2.5.0/gems/rack-1.6.8/lib/rack/lint.rb:49:in `_call'
	/usr/local/lib/ruby/gems/2.5.0/gems/rack-1.6.8/lib/rack/lint.rb:37:in `call'
	/usr/local/lib/ruby/gems/2.5.0/gems/rack-1.6.8/lib/rack/builder.rb:153:in `call'
	/usr/local/lib/ruby/gems/2.5.0/gems/rack-1.6.8/lib/rack/handler/webrick.rb:88:in `service'
	/usr/local/Cellar/ruby/2.5.1/lib/ruby/2.5.0/webrick/httpserver.rb:140:in `service'
	/usr/local/Cellar/ruby/2.5.1/lib/ruby/2.5.0/webrick/httpserver.rb:96:in `run'
	/usr/local/Cellar/ruby/2.5.1/lib/ruby/2.5.0/webrick/server.rb:307:in `block in start_thread'

kt3k · 2018-05-16T11:44:04Z

src/polyfill.js

+// https://github.com/uxitten/polyfill/blob/master/string.polyfill.js
+// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/padEnd
+if (!String.prototype.padEnd) {
+    String.prototype.padEnd = function padEnd(targetLength,padString) {


kt3k · 2018-05-16T15:46:37Z

Thanks for updating! The code looks good to me.

I reproduced the build error of the document with ruby 2.5.0, but I couldn't find the fix. When I changed the ruby version to 2.4.4, it seems able to build. So can you try the ruby 2.4 for building the doc? (I recommend rbenv for version manager of ruby.)

jcsmorais · 2018-05-16T22:26:32Z

@kt3k thanks again for the review.

Your hint on using rbenv made it easy to get documentation builds working, maybe it might be worth to add some notes about using rbenv here so that people like me (that are not as familiar with ruby) can get this working without a problem.

tldr; I updated the PR and added documentation for both x.axis.tick.multiline and x.axis.tick.multilineMax - can you review these please?

Thanks again.

jcsmorais · 2018-05-17T06:51:51Z

@kt3k any chance we can get this one merged in some time soon?

Also, thoughts about releasing a 0.4.23 version with this functionality available for the 0.4.x versions for people that somehow are still stuck on 0.4.22? I'd certainly be willing to backport it.

kt3k · 2018-05-17T07:01:27Z

@jcsmorais
Thanks for adding the doc. It looks perfect now to me 👍

any chance we can get this one merged in some time soon?

Yes. I'm going to merge and release this soon. (Probably today or tommorow)

thoughts about releasing a 0.4.23 version with this functionality available for the 0.4.x versions for people that somehow are still stuck on 0.4.22? I'd certainly be willing to backport it.

Usually we don't back port every feature, but if you can work on it, then I can release it!

kt3k · 2018-05-17T07:07:10Z

I created v0.4 branch for backporting work.

kt3k · 2018-05-17T14:31:12Z

Released as v0.6.1. Thanks for the contribution!

jcsmorais force-pushed the multiline-max branch 3 times, most recently from 92754e9 to 36ef715 Compare May 12, 2018 06:46

jcsmorais force-pushed the multiline-max branch 2 times, most recently from e1de43f to b21656d Compare May 14, 2018 19:31

kt3k reviewed May 15, 2018

View reviewed changes

jcsmorais added 3 commits May 15, 2018 11:04

Add support for axis.x.tick.multiline.max

0f8457a

Add String.padEnd polyfill

7a2aa50

Apply code review suggestions

3528de9

jcsmorais force-pushed the multiline-max branch from dbb9d9e to 3528de9 Compare May 15, 2018 18:06

kt3k reviewed May 16, 2018

View reviewed changes

jcsmorais added 2 commits May 16, 2018 15:18

Add documentation for axis.x.tick.multiline

f88702d

Add documentation for axis.x.tick.multilineMax

0f9f840

jcsmorais changed the title ~~Add support for axis.x.tick.multiline.max~~ Add support for axis.x.tick.multilineMax May 17, 2018

kt3k merged commit 2ca3be3 into c3js:master May 17, 2018

jcsmorais deleted the multiline-max branch May 17, 2018 14:56

This was referenced May 17, 2018

Suggestion: Use ellipsis when running out of space on x-axis #916

Closed

x.axis cuts off if window width size is small and category text exceeds chart height #2341

Closed

jcsmorais added a commit to jcsmorais/c3 that referenced this pull request May 17, 2018

Add support for axis.x.tick.multilineMax backport of c3js#2364 for 0.4.x

ce24e11

jcsmorais mentioned this pull request May 17, 2018

[Backport for 0.4.x] Add support for axis.x.tick.multilineMax #2369

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for axis.x.tick.multilineMax #2364

Add support for axis.x.tick.multilineMax #2364

jcsmorais commented May 10, 2018 •

edited

Loading

codecov-io commented May 10, 2018 •

edited

Loading

jcsmorais commented May 12, 2018

kt3k May 15, 2018 •

edited

Loading

jcsmorais May 15, 2018

kt3k May 15, 2018

jcsmorais May 15, 2018

kt3k commented May 15, 2018

jcsmorais commented May 15, 2018 •

edited

Loading

kt3k May 16, 2018

kt3k commented May 16, 2018

jcsmorais commented May 16, 2018

jcsmorais commented May 17, 2018

kt3k commented May 17, 2018

kt3k commented May 17, 2018

kt3k commented May 17, 2018

Add support for axis.x.tick.multilineMax #2364

Add support for axis.x.tick.multilineMax #2364

Conversation

jcsmorais commented May 10, 2018 • edited Loading

codecov-io commented May 10, 2018 • edited Loading

Codecov Report

jcsmorais commented May 12, 2018

kt3k May 15, 2018 • edited Loading

Choose a reason for hiding this comment

jcsmorais May 15, 2018

Choose a reason for hiding this comment

kt3k May 15, 2018

Choose a reason for hiding this comment

jcsmorais May 15, 2018

Choose a reason for hiding this comment

kt3k commented May 15, 2018

jcsmorais commented May 15, 2018 • edited Loading

kt3k May 16, 2018

Choose a reason for hiding this comment

kt3k commented May 16, 2018

jcsmorais commented May 16, 2018

jcsmorais commented May 17, 2018

kt3k commented May 17, 2018

kt3k commented May 17, 2018

kt3k commented May 17, 2018

jcsmorais commented May 10, 2018 •

edited

Loading

codecov-io commented May 10, 2018 •

edited

Loading

kt3k May 15, 2018 •

edited

Loading

jcsmorais commented May 15, 2018 •

edited

Loading