ember-nf-graph icon indicating copy to clipboard operation
ember-nf-graph copied to clipboard

maxProperty doesn't have a good way to use "default"

Open jamesarosen opened this issue 10 years ago • 17 comments

:smile: If I don't pass a yMax, I get a nice default behavior:

{{ember-nf-graph}}

:smile: If I do pass a yMax, I can get a nice override:

{{ember-nf-graph yMax=100 class='percent-graph'}}

:cry: But there's no way for me to use yMax=someBoundProperty that might be undefined or might be an integer. If it's undefined, then yMax gets set to undefined, not to the default behavior.

I think an easy solution would be to change the logic in the maxProperty to use the override only if it's not null:

// was:
if(arguments.length > 1) {
  this[__Max_] = value;
} else {

// suggested:
if (value != null) {
  this[__Max_] = value;
} else {

jamesarosen avatar Jul 02 '15 18:07 jamesarosen

This is tricky. It seems like a few cases are not covered:

  • null
  • undefined
  • NaN
  • Infinity

I think the behavior you're suggesting is perfect for undefined, where you just don't change whatever it's been set to. But for null or NaN, I think it should probably go to 0 or some minimal number, because it seems like in those cases the consuming code is specifically getting at changing the value to something, even if it's not apparent what that something is.

Thoughts, @jayphelps?

benlesh avatar Jul 06 '15 17:07 benlesh

.. and on the Infinity side, I think that's an extreme edge case, but I can probably default that to MAX_VALUE or MIN_VALUE depending on the polarity of the thing.

benlesh avatar Jul 06 '15 17:07 benlesh

@jamesarosen so I have a good understanding, what's a use case for binding the property, but the value is undefined/null but it is not a bug? I've found myself more and more not doing these sort of "I can't use the value, fall back to default" because they tend to mask bugs in apps that later are hard to track down. Instead, I usually prefer to either throw an actual error or fallback with a console.warn() that the value provided wasn't usable and it's falling back.

jayphelps avatar Jul 06 '15 17:07 jayphelps

My use-case: I'm building a component that wraps nf-graph. It has a unit that could be "percent", "byte", or "count". If it's "percent", then yMax should be 100. Otherwise, it should be auto.

My implementation is

// my-graph.js
yMax: computed("unit", function() {
  return this.get("unit") === "percent" ? 100 : undefined;
})

Alternatively, I could pass in something like "auto" to indicate I want the default behavior.

jamesarosen avatar Jul 06 '15 18:07 jamesarosen

@jamesarosen that seems like a very valid case. I dig your suggestion for "auto" to let nf-graph know to use the default behavior. Back to you @blesh.

jayphelps avatar Jul 06 '15 18:07 jayphelps

In that case, the docs might read something like

yMax: maximum value on the y-axis. Defaults to "auto", which causes the axis to use a value slightly higher than the highest value in the data.

And my example might look something like

{{nf-graph yMax=(if isPercent 100 "auto")}}

jamesarosen avatar Jul 06 '15 18:07 jamesarosen

It's tricky. I sort of view the value the graph was initially set with to be the "default", not necessarily the coded default. It seems like we could memoize that when it's initialized, somehow. I think passing undefined and expecting it to default is fine. I'm more concerned with that matching up with where someone is inadvertently passing in NaN due to some miscalculation, I guess.

Perhaps it should be like this:

{{#nf-graph xMax=someValue xMaxDefault=100}}

Where if someValue is undefined or null it gets set to xMaxDefault...

However, in the event that someValue is NaN it should be set to 0 and MIN_VALUE or MAX_VALUE for ±Infinity.

Thoughts?

benlesh avatar Jul 06 '15 18:07 benlesh

What's the current behavior if I set yMax=NaN?

jamesarosen avatar Aug 26 '15 19:08 jamesarosen

It should behave the same as a 0, I think.

benlesh avatar Aug 26 '15 22:08 benlesh

So you want

  • undefined, null, NaN, 0 to all be treated as "calculate max"
  • any number to be treated as a fixed value

jamesarosen avatar Sep 14 '15 17:09 jamesarosen

I really depends on the yMaxMode how it will behave. If it's auto it should ignore setting the value entirely.

benlesh avatar Sep 14 '15 18:09 benlesh

@blesh can you give me very specific requirements? I need to work on this this week to ship a feature. I can make up an implementation that works for me, but I'd rather do something that will get merged here (eventually).

jamesarosen avatar Sep 14 '15 18:09 jamesarosen

different yMaxModes:

  • auto: automatically calculate yMax from contained graphics' data
  • push: allow setting of yMax, but push the yMax value up if the contained data passes it.
  • push-tick: same as push, but go to the next nicest "tick" value.
  • fixed: user determines the yMax value at all times.

When in doubt, use fixed and set it yourself. We do this for things like sliding windows, etc.

Does that help?

benlesh avatar Sep 14 '15 18:09 benlesh

These yMaxModes suggest maybe I'm going about this wrong. Imagine the following use-case:

When showing a percentage graph, default to showing [0-100%] by default. Allow the user to zoom in on the displayed range.

Given that, I would guess something like

{{nf-graph yMax=100
           yMin=0
           yMaxMode=(if isZoomed "push" "fixed")
           yMinMode=(if isZoomed "auto" "fixed")}}

jamesarosen avatar Sep 14 '15 18:09 jamesarosen

I'm totally cool with setting yMax to "auto" or some other string when I want default behavior:

{{nf-graph yMax=(if isZoomed "auto" 100)}}

jamesarosen avatar Sep 17 '15 21:09 jamesarosen

The method that is going to give you the most control will be using fixed in most cases. auto and push or push-tick were really added as conveniences, but might not be doing the most efficient thing when trying to suss out where your min and max values lie.

benlesh avatar Sep 17 '15 21:09 benlesh

but might not be doing the most efficient thing when trying to suss out where your min and max values lie.

For example, maybe your array is already sorted, or maybe your service already gave you the min and max values, and you can just pick them out of an object or an array without having to iterate over that array.

benlesh avatar Sep 17 '15 21:09 benlesh