Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename EuiXYChart to EuiAxisChart, and associated components #1050

Closed
wants to merge 0 commits into from

Conversation

cjcenizal
Copy link
Contributor

I talked this over with @mattapperson. I think EuiAxisChart is a little more intuitive, and will also cohere nicely with the axis components he's built. It also has the side effect of being consistent with the camel-cased way the other components are named, i.e. XY would be written as Xy for the same reason EUI is written as Eui, the result being EuiXyChart which looks a little weird.

CC @elastic/kibana-design

@snide
Copy link
Contributor

snide commented Jul 25, 2018

Makes sense to me. Prolly want @markov00 is ok too.

@cjcenizal cjcenizal requested a review from markov00 July 25, 2018 22:22
@cchaos
Copy link
Contributor

cchaos commented Jul 25, 2018

Can we get #1041 merged first to avoid conflicts later?

@cjcenizal
Copy link
Contributor Author

@cchaos Sure no problem

@markov00
Copy link
Member

markov00 commented Jul 30, 2018

@cjcenizal from my point of view it's a bit confusing using the word axis for naming that chart component since it's refers to an existing components. You can ends up writing something like:

<EuiAxisChart>
  <EuiXAxis />
  <EuiYAxis />
  <EuiBarSeries />
</EuiAxisChart>

The nature of the XY in the name was because recalls the X and Y variables thats usually oriented along the horizontal and vertical planes, the opposite of other type of charts thats have always X and Y variables but are placed in a radial fashion (pie chart, chord diagram, polar charts etc) or without a XY set of planes like in circle packing chart, gauge chart etc.

These are the naming that are adopted by some charting libraries:

Maybe it could be something more generic like EuiChart or EuiPlotChart.

@timroes what do you think?

@mattapperson
Copy link
Contributor

@markov00 I think this creates no more confusion then EuiXAxis where X is in the name of the chart component. Also AxisChart is more accurate IMHO because there can be many Axis in this component (true all are X and Y), but to me this is much clearer and easy to understand.

For the back-story of naming this I will provide the following options that were considered and why they were passed on:

  • Just Chart. Passed on because we plan to support more then just this style of chart, and supporting all the underpinnings for all the types we would support in a single component would have made for hard to maintain code.
  • 'GridChart` Closer to reality, but perhaps not the most accurate, felt it did not communicate properly what this was
  • Plot This one almost one, but someone (I forget who) said this was hard to understand, so I passed.

@markov00
Copy link
Member

All charts have axis and their axis can be relative to each single graphic object it represent or general to the whole chart. Having a EuiAxisChart can be seen as any chart that can be represented by 1 or n axis.
With the EuiXYChart we are defining the container component that handles the configuration for the chart. In the current implementation it manage the rendering of series that can be displayed in a two dimensional rectangular coordinate system.

So after few other thoughts I think the most meaningful name can be EuiCartesianChart that represent correctly the cartesian coordinate system used by all the implemented series. What about that?

@mattapperson
Copy link
Contributor

@markov00 all good points, though I think perhaps Cartesian is not a well known term? A few others I would throw out there SeriesChart or DefaultChart (as I think most people would think of this type of chart as a "typical" chart And anything else is more commonly referred to by name?

@cchaos
Copy link
Contributor

cchaos commented Jul 30, 2018

If it helps at all, I like the idea (in the new vis builder) of calling these "Basic" chart types.

@chandlerprall
Copy link
Contributor

though I think perhaps Cartesian is not a well known term?

Great way for people to learn! :) Especially since the docs point to the component; no one has to hunt around through source files to find what they need.

I would learn toward reserving canonicalizing basic/simple/etc chart types for more building-block style, less user-configurable (like the existing table -> basic table -> in memory table "stack")

@cjcenizal
Copy link
Contributor Author

When choosing a name for a component which interacts regularly with other components, I think one which implies this cohesion is best. I looked through the examples again and found that, aside from the axis components, the chart is consistently used with the series components. I think EuiSeriesChart will reinforce this relationship, especially since this type of chart is essentially useless without a series component:

<EuiSeriesChart>
  <EuiAreaSeries />
  <EuiLineSeries />
  <EuiXAxis />
  <EuiYAxis />
</EuiSeriesChart>

This has precedent in Kibana, in which the "point series" domain forms a core part of the vis library. The similar name will make it seem natural to integrate this component into that domain.

If nobody objects by tomorrow, I'll make this change and force-push this branch, probably overwriting the original commit since there are so many merge conflicts.

@monfera
Copy link

monfera commented Jul 30, 2018

@chandlerprall You give reasons both for and against Cartesian, and I'd lean for the for bit, because Cartesian is precise, and something like "plain", "basic" or "default" doesn't say what aspect the default-ness refers to. "Cartesian" is not a universally remembered term from school, so "XY"-whatever sounds like a good compromise, although if I'm not mistaken, these are just component names so the audience for the name is software developers and fairly technical solution builders who probably don't mind Cartesian.

I'm not familiar with the convention but while EUI is an abbreviation, XY is not an abbreviation, X and Y are simply one-character words 😆 So I'm fuzzy on why "Xy" would be more compliant than "XY".

Anything that's camelCased is probably a technical term, and for technical terms, precision and meaning have various benefits, for example, stability over time.

We'll also run into scalability. Let's take "linear" vs non-linear eg. logarithmic. The term "linear" is also taught at school and also not universally remembered. We could have eg. "basic" vs "logarithmic" plots to avoid using the more obscure term "linear". But then "basic" starts to assume an undefined medley of meanings (of course, linear vs not is more of an axis property so pick your example).

@markov00
Copy link
Member

Ok, I think we had enough discussion and material on that, what about to poll on this?

😄 EuiXYChart: keep the old name
🎉 EuiAxisChart
😕 EuiCartesianChart
❤️ EuiSeriesChart

Each one can express 1 or more preference

@cjcenizal
Copy link
Contributor Author

cjcenizal commented Jul 31, 2018

XY is not an abbreviation, X and Y are simply one-character words

I never considered this! I thought it was an abbreviation for "X-axis Y-axis". I don't know what the words "X" or "Y" mean on their own... 😄

I think everyone here has brought up really good points. We've identified some criteria for a good name:

  1. It implies the horizontal and vertical dimension of the chart (Marco's point)
  2. Something people can easily understand (Matt's point)
  3. Doesn't impinge on the primitive/basic/advanced naming pattern (Chandler's point)
  4. Something which implies cohesion with its related components (My point)

To move forward we don't need a decision which everyone is happy with but we need one which nobody is unhappy with. I feel strongly (8 on a scale of 10) that EuiSeriesChart is the best choice because it meets our above criteria. Most importantly I think it will make the code easily understandable. When you offer a solution with a lot of moving parts, people will be able to pick it up more quickly if those parts look like they fit together.

So to avoid blocking this on more discussion: does anybody hate this name?

@monfera
Copy link

monfera commented Jul 31, 2018

We've identified some criteria for a good name:

  1. It implies the horizontal and vertical dimension of the chart (Marco's point)

I thought this criterion was important, and this name doesn't imply a Cartesian, orthogonal or x/y nature. A series can be projected onto a radial coordinate system, carpet plot, ternary plot, parallel coordinates or pretty much anything.

Yet I can't get worked up about naming, and "series" is a cool word!

@cjcenizal
Copy link
Contributor Author

@monfera Very true! We do lose a bit of information with the name. We can differentiate the other chart types you mention by adding more info to the name: EuiSeriesRadialChart, EuiSeriesCarpetChart, EuiSeriesTernaryChart. EuiSeriesChart would stand out as the simplest name. Maybe that will reflect its role as the most commonly-used kind of chart? Or maybe that's a stretch. 😄

@chandlerprall
Copy link
Contributor

I really like Cartesian as it is explicit and doesn't imply anything. It requires knowing a word, but isn't ambiguous like Series. That said, I'd be fine with Series.

@monfera
Copy link

monfera commented Jul 31, 2018

Just so I learn a bit in the process, what's EuiSeriesChart going to contrast to? A quick grep on Eui[A-Za-z]*Chart didn't yet show other types?

What's the motive and planned boundary for disambiguating things via names, vs using just properties? The projection can be just a prop projection or type which, when unspecified, defaults to cartesian. What's the benefit of not just naming it EuiChart? Why is projection deemed as the key disambiguator? For example, another componentization scheme could be the layout: EuiChart for simple, one-panel charts, and EuiSmallMultiplesChart for small multiples etc.

In short, what's the larger thing being designed and what are its constraints and goals?

Maybe this gh issue could be called, "design a charting nomenclature / catalog with goals 1, 2, 3", unless such a design already exist. Or maybe a full visualization grammar is being informally designed, eg. how users specify per-axis properties, presence of overlays (boxplot, candles, outliers, text annotations, error bars, densities, jitter, ...), interactivity (selection, linked brushing...) and everything else, learning from Plotly, Vega etc.?

@cjcenizal
Copy link
Contributor Author

I messed up the force-push so I just opened a new PR to replace this one: #1066

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants