egGrid row styling: code design questions

[Dan Wells]

On bug #1635386, we added a new attribute to egGrid called rowClass. This is an '=' bound attribute which references an object containing an "apply()" function for applying different CSS classes to rows. This bug is partly to plant a flag in the ground for myself to investigate a few things, but also to seek wisdom of those who have gone before into deeper AngularJS brush.

Questions I hope to answer:
1) Is there any particular reason to have rowClass be an object with single function member, rather than just a function itself?
2) If we are just invoking a function on the controller scope, perhaps "&" binding would be better?
3) Perhaps related to #1, we currently call rowClass.apply() in an ng-class array, even though in most cases it won't exist. I haven't seen any errors. Are errors being generated? If so, where are these errors going? If not, is this normal Angular practice to be fast and loose with calling undefined functions in directive attributes, or should we be more conservative here, and somehow wrap this call ourselves?
4) We have, for example, two attributes, idlClass and rowClass. One is "@" bound, the other "=" bound, so they behave totally differently, but look basically the same in the template. Is there some way (without inspecting the underlying JS) for the template writer to know what to pass into the attribute, whether a string or an object name? Or is there a best practice for naming attributes with different bindings to make things more clear?
5) In this particular case, do we want to stick with "rowClass" as the name? I think it is easy for someone to assume this is supposed to be CSS classname, not the name of an object with code to classify rows and return CSS classnames. Perhaps "rowClasser", "rowClassFunc", or something of that nature would clear things up?

My initial answers are:
1) No.
2) Maybe?
3) ???
4) ???
5) Yes.
These are subject to trying things out and perhaps rediscovering why the current hoops exist.

[Bill Erickson (berick)]

On Thu, Jun 28, 2018 at 4:55 PM Dan Wells <email address hidden> wrote:
<snip>

> Questions I hope to answer:
> 1) Is there any particular reason to have rowClass be an object with
> single function member, rather than just a function itself?
>

Agreed a bare function is fine.

> 2) If we are just invoking a function on the controller scope, perhaps "&"
> binding would be better?
>

Since the grid is invoking rowClass(), an '&' binding is probably more
correct (and efficient).

> 3) Perhaps related to #1, we currently call rowClass.apply() in an
> ng-class array, even though in most cases it won't exist. I haven't seen
> any errors. Are errors being generated? If so, where are these errors
> going? If not, is this normal Angular practice to be fast and loose with
> calling undefined functions in directive attributes, or should we be more
> conservative here, and somehow wrap this call ourselves?
>

AngularJS is very forgiving of exceptions in template expressions. No
errors are logged that I'm aware of. It behaves more or less like: [ try {
eval(exp) } catch (err) { return "" } ] (Though, to be clear, it's not
using eval() under the covers).

Angular(.io) will throw big noisy exceptions for null refs, etc.

> 4) We have, for example, two attributes, idlClass and rowClass. One is
> "@" bound, the other "=" bound, so they behave totally differently, but
> look basically the same in the template. Is there some way (without
> inspecting the underlying JS) for the template writer to know what to pass
> into the attribute, whether a string or an object name? Or is there a best
> practice for naming attributes with different bindings to make things more
> clear?
>

There's no way to tell in AngularJS just by looking at the attribute name.
Sometimes you can tell by the existing value. Usually, you have to inspect
the component.

Angular(.io) resolves this by supporting 3 types of component attributes:
abc="string", [abc]="value", (abc)="eventHandler"

> 5) In this particular case, do we want to stick with "rowClass" as the
> name? I think it is easy for someone to assume this is supposed to be CSS
> classname, not the name of an object with code to classify rows and return
> CSS classnames. Perhaps "rowClasser", "rowClassFunc", or something of that
> nature would clear things up?
>

Agreed the name could be more descriptive. I have no strong preference,
but FWIW following along with this bug while working on the Angular6 grid,
I have rowClassCallback (and rowFlairCallback for in-row alert icons).

-b

[Stephanie Leary (stephanieleary)]

Do we need additional work on row classes, or can we close this bug?