Skip to content

Commit

Permalink
Add warning for keyless use of SortableFactory
Browse files Browse the repository at this point in the history
This less-than-elegant inclusion of the warning in the computed
property is necessary because the mixing-in happens as the
application is initialised, so having it outside of the computed
property causes it to unhelpfully with no context.
  • Loading branch information
backspace committed Dec 10, 2019
1 parent a8c571e commit 0131c36
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
17 changes: 16 additions & 1 deletion ui/app/mixins/sortable-factory.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Mixin from '@ember/object/mixin';
import { computed } from '@ember/object';
import { warn } from '@ember/debug';

/**
Sortable mixin factory
Expand All @@ -16,7 +17,7 @@ import { computed } from '@ember/object';
Properties provided:
- listSorted: a copy of listToSort that has been sorted
*/
export default function sortableFactory(properties) {
export default function sortableFactory(properties, fromSortableMixin) {
const eachProperties = properties.map(property => `listToSort.@each.${property}`);

return Mixin.create({
Expand All @@ -25,12 +26,26 @@ export default function sortableFactory(properties) {
sortDescending: true,
listToSort: computed(() => []),

_sortableFactoryWarningPrinted: false,

listSorted: computed(
...eachProperties,
'listToSort.[]',
'sortProperty',
'sortDescending',
function() {
if (!this._sortableFactoryWarningPrinted) {
let message =
'Using SortableFactory without property keys means the list will only sort when the members change, not when any of their properties change.';

if (fromSortableMixin) {
message += ' The Sortable mixin is deprecated in favor of SortableFactory.';
}

warn(message, properties.length > 0, { id: 'nomad.no-sortable-properties' });
this.set('_sortableFactoryWarningPrinted', true);
}

const sorted = this.listToSort.compact().sortBy(this.sortProperty);
if (this.sortDescending) {
return sorted.reverse();
Expand Down
2 changes: 1 addition & 1 deletion ui/app/mixins/sortable.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ import SortableFactory from 'nomad-ui/mixins/sortable-factory';

// A generic version of SortableFactory with no sort property dependent keys.

export default SortableFactory([]);
export default SortableFactory([], true);

0 comments on commit 0131c36

Please sign in to comment.