angulardart / angular_components

The official Material Design components for AngularDart. Used at Google in production apps.
https://pub.dev/packages/angular_components
374 stars 123 forks source link

Different types for [tabindex] in material-toggle and material-button #412

Open kankaristo opened 5 years ago

kankaristo commented 5 years ago

I'm setting tabindex in the template with [tabindex]="someVariable" for some elements.

<material-toggle> expects an int for [tabindex], but <material-button> wants a String. I haven't tested other elements yet, those are the ones I've run into so far.

I'd argue that the correct type is int (since it is an integer, not a string), but I don't really care as long as it would be the same for all elements. Now I have to use separate variables, or a property that converts the integer to a string.

nshahan commented 5 years ago

There are some inconsistencies between component APIs since this group of widgets was developed organically over time. We would like to fix issues like this but it comes at a high cost of updating all of the client applications using the code so we focus more on consistency when adding new components and features.

supermuka commented 5 years ago

Just an observation/suggest. For years I have been following the evolution of some languages ​​and tools, recently and more closely Dartlang as well as AngularDart. It is natural to have inconsistent design patterns between features. However, for a language or library to be popular, it is easy to maintain for its evolution, it is important to eliminate these inconsistencies, usually through the deprecated (@deprecated) or migration tutorial, thus minimizing the cost of updating.

kankaristo commented 5 years ago

I agree with @supermuka. You don't want these "quirks" building up over time, or you'll have a mess of a language/framework on your hands, that no-one wants to use. AngularDart is already struggling with popularity a bit.

And if I remember correctly, at least dartanalyzer only shows this as a warning, not an error. So, changing the type might not actually break existing applications, only show new warnings.