fix(ts): make onChange argument optional

[lukasnys]

In the addon/src/modifiers/sortable-group.ts file, the onChange argument is marked as required. However, looking at the implementation, it looks like it should be optional instead.

[netlify]

✅ Deploy Preview for ember-sortable canceled.

Name	Link
🔨 Latest commit	1e6dd01
🔍 Latest deploy log	https://app.netlify.com/sites/ember-sortable/deploys/66f3c897dc6d1e000852eb4d

[NullVoxPopuli]

What's the usecase for not having @onChange?

[mkszepp]

i think the only usecase for optional with latest is when there is disabled=true or? because without onChanges the data will never be changes

Looking into code docs there is declared as optional, but maybe its historcally, when there was 2-way binding... is it possible?

ember-sortable/addon/src/modifiers/sortable-group.ts

Lines 59 to 78 in 0e28f8b

	/**
	* Modifier to apply a11y support to a group container for the Sortable component
	*
	* @param {String} [a11yItemName] A name for each model, used for creating more meaningful a11y announcements.
	* @param {Object} [a11yAnnouncementConfig] A map of action to function to build meaningful a11y announcements.
	* @param {String} [itemVisualClass] A class for styling visual indicators on the yielded `sortable-item`.
	* @param {Object} [handleVisualClass] An object for styling visual indicators on the yielded `sortable-handle` on different `move`.
	* @param {Function} [onChange] An optional callback for when position rearrangements are confirmed.
	*
	* @module drag-drop/draggable-group
	* @example
	* <ol {{sortable-group onChange=this.update a11yAnnouncementConfig=this.myA11yConfig}}>
	* {{#each model.items as |item|}}
	* <li {{sortable-item model=item}}>
	* {{item.name}}
	* <span class="handle" {{sortable-handle}}>&varr;</span>
	* </li>
	* {{/each}}
	* </ol>
	*/

I think in real cases you need always to pass a onChange

[lukasnys]

You both make good points. I saw a usage in our code base where we don't pass a onChange so I figured it was a valid use case. Turns out this makes for some really weird and un-user-friendly behaviour so I'm getting rid of it.

Closing this PR as it indeed doesn't make sense!