Tachyons (CSS) Incompatibility

[caseywatts]

Billboard.js currently doesn't work in projects that use Tachyons.

Here is a fiddle with Tachyons included (it doesn't render)
https://jsfiddle.net/moy6420x/

Here is the same fiddle, but without Tachyons (it does render):
https://jsfiddle.net/Lbdrf77f/

The issue is probably a conflict with the Tachyons CSS class .bb (~= border bottom). I'm not even sure a good workaround for this yet.

[netil]

Thanks @caseywatts for reporting the issue.
I'll try to look for it :)

[caseywatts]

I poked around the source code a bit - I'm having trouble telling what the .bb class is responsible for and where it gets assigned. It seems like it's usually something like a ~billboardjs-container?

[netil]

All the class names used are defined at

• https://github.com/naver/billboard.js/blob/master/src/config/classes.js

The below image is the basic DOM tree constructed. You can see that class names with the prefix bb- are defined according its role.

The basic idea on providing default css file is not to use without change. It means, change css according your needs.

So, when you need to combine with other css frameworks/libraries, perhaps changes are required to prevent some unexpected styling.

[caseywatts]

Thanks for looking into this with me @netil !

All of the css class names in that file classes.js are good and don't/wouldn't clash with Tachyons or others. I like that they're namespaced bb-, that's great :)

At the root of the DOM image you pasted is id='LineChart' class='bb', and that bb there seems like it might be the conflict? There is some css in the bundled css output that depends on things being nested within something with class bb. That's the part that does conflict with Tachyons and seems like it would more likely conflict with other frameworks (not classes namespaced with bb-).

Could you help me find where in the codebase bb is applied to the root element?

[netil]

Well, the class name bb is set to base element at the time of instantiation.

billboard.js/src/internals/ChartInternal.js

Line 188 in b2ad2a5

$$.selectChart.html("").classed("bb", true);

and for default css file, you can find the definition as below.

billboard.js/src/scss/chart.scss

Lines 1 to 17 in b2ad2a5

	.bb {
	svg {
	font: 10px sans-serif;
	-webkit-tap-highlight-color: rgba(0,0,0,0);
	}
	path, line {
	fill: none;
	stroke: #000;
	}
	text {
	-webkit-user-select: none;
	-moz-user-select: none;
	user-select: none;
	}
	}

[caseywatts]

awesome! This is just what I needed 🎉

I changed those two instances of bb to billboard-chart locally, and everything works as expected now :)

I'd be happy to make a PR to change this bb to something more descriptive?

Some ideas:

• billboardjs
• billboardjs-chart
• billboard-chart
• bb-chart-container
• bb-container

[netil]

@caseywatts I think we need discuss more on this, because changing the class name can affect in various way like compatibility.

And also I don't think that bb is kinda common name which is broadly used and having some incompatibility issue always.

Change is easy, but if is possible we need to find a way to minimize those affects for previous releases.

[netil]

I think there're 2 alternatives for this.

1. An option to set the class name prefix.
   In this case, the bb-somthing will be replaced as bb-chart-something.

bb.generate({
     prefix: "bb-chart",
     ...
});

2. Or set a global option, which can affect all of the generation.

// Just set prefix once, and maintain the same configuration
bb.config({
     prefix: "bb-chart"
});

// All generation will have 'bb-chart-something' class name.
bb.generate(...);
bb.generate(...);

[caseywatts]

Thanks for thinking about this more :)

I can imagine:

bb.generate({
    container-class: 'bb-chart',
    prefix: 'bb-chart'
    ...
})

As for option 1 vs 2 - either one would work. I imagine 1 is easier to implement, so we can go with that one~

Also, to reiterate

• I don't need to be able to adjust the prefix that's used in all of the classes in https://github.com/naver/billboard.js/blob/master/src/config/classes.js
• I do need to be able to adjust the container-class that's used in
  

  billboard.js/src/internals/ChartInternal.js

  Line 188 in b2ad2a5

  $$.selectChart.html("").classed("bb", true);

[caseywatts]

Once we implement this change, I can also make a PR to the readme and/or docs site to warn users of Tachyons that they need to use this option (or it would fail silently).

[netil]

I'll try implement this as follow.
Using the existing bindto option, expanding it to allow set detail option for wrapper element.

// it can be used as it was
bindto: "#chart",

// or set more specifically
bindto: {
	element: "#chart",
	classname: "classname-value"
}

[caseywatts]

This bindto solution does allow me to avoid having bb be present, and it gives us backward compatibility nicely. This will technically unblock me, but it still might be tough for anyone else in this situation. Two reasons, and an idea for another path forward:

base css (currently .bb)

With this bindto solution, we'll also want to add more about css to the docs. We can point users to these 17 lines of js and explain that they'll likely need to duplicate them for their custom class (mostly the middle 4)

Without that .bb css, the charts would look like this:

custom class names for the billboard container?

I don't need to be able to customize the classname, other than to avoid it being bb lol.

I'm comfortable putting any custom classname I need additionally on the same element I use as the bindto. The ember-c3 wrapper that I'm using (I modified it locally so I can work with billboard in the same way) - it puts a custom css attribute on the container, too.

another idea?

(I'm not 💯 that this is best either, I'm just adding ideas to our pool of options~)

I suspect it might be easier / more efficient long-term if we replace bb with something less likely to clash, like billboard-chart. But that is tough to do while maintaining backwards compatibility. I could imagine a migration path:

• introduce a flag that can be set that switches bb to billboard-chart for users ready/willing to use the new way (in particular, any Tachyons users)
• introduce a deprecation message that bb will be changing to billboard-chart (idk where this could go - in the readme? and also after installation?)
• eventually (in 6 mo or a year?) a "major" release happens that swaps this out as the default - this can be in the release notes for that major version

---

I have to say thank you again, for being so active at maintaining billboard.js and discussing feedback openly - I don't take this for granted lol
🎉 🤗

[netil]

Thank @caseywatts for your thoughts and really appreciate that.

I think those users having this necessities are users who has good knowledge on handling this.
So, the concerns(which needs to update css when classname changes) isn't really an issue.

I'm seeing that giving the chance to update classname is enough for this moment, but this doesn't mean not having any changes in the future.

The nature of opensource is always hearing the thoughts and opinions from the users, adopting and evolving base on that :)