search

angular / components

Component infrastructure and Material Design components for Angular
https://material.angular.io
MIT License
24.22k stars 6.69k forks source link

Component *change events consistency #7191

Open rafaelss95 opened 6 years ago

rafaelss95 commented 6 years ago

Bug, feature request, or proposal:

Proposal.

What is the expected behavior?

I'd expect all components to have the same behavior. Or all return a plain object, or all return an instance of a class.

What is the current behavior?

Some components return a plain object, others return an instance of a class.

What components have a different behavior?

Topic 1 (About emit a plain object):

Chip: https://github.com/angular/material2/blob/5210b3e197647381611ec48ddcd99333ec5356e9/src/lib/chips/chip.ts#L183

Paginator: https://github.com/angular/material2/blob/5210b3e197647381611ec48ddcd99333ec5356e9/src/lib/paginator/paginator.ts#L189

Sort: https://github.com/angular/material2/blob/5210b3e197647381611ec48ddcd99333ec5356e9/src/lib/sort/sort.ts#L131

Topic 2 (Some "change" events are named ...Event, some others are ...Change and others are ...ChangeEvent): https://github.com/angular/material2/blob/5210b3e197647381611ec48ddcd99333ec5356e9/src/lib/autocomplete/autocomplete.ts#L35 Suffixed by 'Event'.

https://github.com/angular/material2/blob/5210b3e197647381611ec48ddcd99333ec5356e9/src/lib/tabs/tab-group.ts#L43 This is the only event that is suffixed by 'Change' and 'Event'.

https://github.com/angular/material2/blob/5210b3e197647381611ec48ddcd99333ec5356e9/src/lib/chips/chip.ts#L25 Besides being an interface (not a class), it isn't suffixed by 'Change'.

https://github.com/angular/material2/blob/5210b3e197647381611ec48ddcd99333ec5356e9/src/lib/chips/chip-input.ts#L16 Not sure about this. I think this can't be considered inconsistency, but I'll left it here anyway.

https://github.com/angular/material2/blob/5210b3e197647381611ec48ddcd99333ec5356e9/src/lib/paginator/paginator.ts#L30 Suffixed by 'Event'.

https://github.com/angular/material2/blob/5210b3e197647381611ec48ddcd99333ec5356e9/src/lib/sort/sort.ts#L41 Besides being an interface (not a class), it isn't suffixed by 'Change'.

Is there anything else we should know?

Maybe it's a complement of #6677.

amcdnl commented 6 years ago

This is being partly addressed by https://github.com/angular/material2/issues/6677

rafaelss95 commented 6 years ago

Hey @amcdnl, as I said it's a complement of #6677.

There are inconsistencies here that aren't pointed out in that issue.

And, according to your last comment there you've already done all the work for that issue.

vivian-hu-zz commented 5 years ago

Team discussed the options of

  1. Omitting the word "Event" from the names and instead suffixing with the type of event (e.g. "Change").
  2. Always put "Event" at the end The decision is to go with option 2: always put "Event" at the end. It will be clearer to have "MatDatepickerInputEvent" than "MatDatepickerInput"
rafaelss95 commented 5 years ago

@vivian-hu Hmm.. any chance to do this before v8?

jelbourn commented 5 years ago

This won't make it in for v8

rafaelss95 commented 3 years ago

@jelbourn Any news about this? I would love to help with this issue once you tell me the team decision about the convention.

jelbourn commented 3 years ago

Vivian had been the one owning this project, but she left the team a long while ago, and it basically fell to the wayside when that happened. With everything else we've got going on now, I don't think this will be on our radar in 2020.