Harej has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/351094 )
Change subject: Change JS list editor to use numeric indices instead of titles.
......................................................................
Change JS list editor to use numeric indices instead of titles.
A page title could be included on a list multiple times, and this
causes issues with the JS list editor. This change assigns "item
IDs" to each list item, based on the column and item index, and
changes all list editor behavior to use those instead of the page
title.
Change-Id: I01af89264c59f943274c422b630b1f9fb09622ca
---
M extension.json
M includes/content/CollaborationListContent.php
M modules/ext.CollaborationKit.list.edit.js
M modules/ext.CollaborationKit.list.ui.js
4 files changed, 105 insertions(+), 101 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CollaborationKit
refs/changes/94/351094/1
diff --git a/extension.json b/extension.json
index 9a9f0ac..005c968 100644
--- a/extension.json
+++ b/extension.json
@@ -197,6 +197,7 @@
"collaborationkit-list-delete-summary",
"collaborationkit-list-delete-popup",
"collaborationkit-list-delete-popup-title",
+ "collaborationkit-list-error-editconflict",
"collaborationkit-list-error-saving",
"collaborationkit-list-move",
"collaborationkit-list-move-summary",
diff --git a/includes/content/CollaborationListContent.php
b/includes/content/CollaborationListContent.php
index c60c1c7..871665f 100644
--- a/includes/content/CollaborationListContent.php
+++ b/includes/content/CollaborationListContent.php
@@ -342,6 +342,7 @@
$sortedItems = $column->items;
$this->sortList( $sortedItems, $options['defaultSort']
);
+ $itemCounter = 0;
foreach ( $sortedItems as $item ) {
if ( $offset !== 0 ) {
$offset--;
@@ -366,8 +367,10 @@
$text .= Html::openElement( 'div', [
'style' => "min-height:{$iconWidth}px",
'class' => 'mw-ck-list-item',
- 'data-collabkit-item-title' =>
$item->title
+ 'data-collabkit-item-title' =>
$item->title,
+ 'data-collabkit-item-id' => $colId .
'-' . $itemCounter
] );
+ $itemCounter++;
if ( $options['mode'] !== 'no-img' ) {
if ( isset( $item->image ) ) {
$text .= static::generateImage(
diff --git a/modules/ext.CollaborationKit.list.edit.js
b/modules/ext.CollaborationKit.list.edit.js
index 8865022..d1409bd 100644
--- a/modules/ext.CollaborationKit.list.edit.js
+++ b/modules/ext.CollaborationKit.list.edit.js
@@ -4,7 +4,7 @@
( function ( $, mw ) {
'use strict';
- var deleteItem, getCurrentJson, saveJson, reorderList, getListOfTitles,
getColId;
+ var deleteItem, getCurrentJson, saveJson, reorderList, getListOfItems,
getColId, renumberElements;
/**
* Retrieves ID number of column
@@ -24,13 +24,28 @@
};
/**
+ * Renumbers the item IDs within a column following a re-ordering or a
deletion
+ *
+ * @param {number} colId the column in which to re-number the items
+ */
+ renumberElements = function ( colId ) {
+ $( '.mw-ck-list-column[ data-collabkit-column-id=' + colId + '
] .mw-ck-list-item' )
+ .each( function ( index ) {
+ $( this ).data( 'collabkit-item-id', colId +
'-' + index );
+ } );
+ }
+
+ /**
* Deletes an item from the list
*
* @param {jQuery} $item
*/
deleteItem = function ( $item ) {
- var spinner,
+ var i,
+ oldItems,
+ spinner,
title = $item.data( 'collabkit-item-title' ),
+ itemId = $item.data( 'collabkit-item-id' ),
colId = getColId( $item );
if ( mw.config.get( 'wgCollaborationKitIsMemberList' ) ) {
@@ -48,12 +63,13 @@
getCurrentJson( mw.config.get( 'wgArticleId' ), function ( res
) {
var newItems = [];
- $.each( res.content.columns[ colId ].items, function (
index ) {
- if ( this.title === title ) {
- return;
+ oldItems = res.content.columns[ colId ].items;
+ for ( i = 0; i < oldItems.length; i++ ) {
+ if ( colId + '-' + i === itemId ) {
+ continue;
}
- newItems[ newItems.length ] = this;
- } );
+ newItems[ newItems.length ] = oldItems[ i ];
+ };
res.content.columns[ colId ].items = newItems;
// Interface for extension defined tags lacking...
// res.tags = 'collabkit-list-delete';
@@ -61,6 +77,7 @@
res.summary = mw.msg(
'collaborationkit-list-delete-summary', title );
saveJson( res, function () {
$item.remove();
+ renumberElements( colId );
mw.notify(
mw.msg(
'collaborationkit-list-delete-popup', title ),
{
@@ -69,7 +86,6 @@
type: 'info'
}
);
-
} );
} );
};
@@ -77,19 +93,18 @@
/**
* Helper function to get ordered list of all items in list
*
- * @param {jQuery} $elm The list of items - $( '.mw-ck-list' )
+ * @param {jQuery} $elm The list of items
* @return {Array} 2D array of all items in all columns.
*/
- getListOfTitles = function ( $elm ) {
+ getListOfItems = function ( $elm ) {
var list = [];
- // FIXME must be changed for multilist.
$elm.children( '.mw-ck-list-column' ).each( function () {
var $this, colId;
$this = $( this );
colId = $this.data( 'collabkit-column-id' );
list[ colId ] = [];
$this.children( '.mw-ck-list-item' ).each( function () {
- list[ colId ][ list[ colId ].length ] = $( this
).data( 'collabkit-item-title' );
+ list[ colId ][ list[ colId ].length ] = $( this
).data( 'collabkit-item-id' );
} );
} );
return list;
@@ -112,19 +127,24 @@
getCurrentJson( mw.config.get( 'wgArticleId' ), function ( res
) {
var i,
j,
+ moveFrom,
+ moveTo,
+ movingItem,
+ newPosition,
+ oldPosition = $item.data( 'collabkit-item-id' ),
reorderedItem,
- findItemInResArray,
- resArray,
+ resColumns,
isEditConflict = false;
reorderedItem = $item.data( 'collabkit-item-title' );
+ // Edit conflict detection
outer: for ( i = 0; i < originalOrder.length; i++ ) {
if ( res.content.columns[ i ].items.length !==
originalOrder[ i ].length ) {
isEditConflict = true;
} else {
for ( j = 0; j < originalOrder[ i
].length; j++ ) {
- if ( res.content.columns[ i
].items[ j ].title !== originalOrder[ i ][ j ] ) {
+ if ( i + '-' + j !==
originalOrder[ i ][ j ] ) {
isEditConflict = true;
break outer;
}
@@ -134,7 +154,7 @@
if ( isEditConflict ) {
// FIXME sane error handling.
- alert( 'Edit conflict detected. Rearrangement
not saved.' );
+ alert( mw.msg(
'collaborationkit-list-error-editconflict' ) );
location.reload();
throw new Error( 'Edit conflict' );
}
@@ -148,69 +168,36 @@
throw new Error( 'Lost an item in the list?!' );
}
- /**
- * Find an item in the result array.
- *
- * Optimized to first look in the most likely spots.
- * Assumes that titles must be unique in a list.
- *
- * @param {string} title Title of list item to find
- * @param {number} indexGuess Where we think it might be
- * @param {number} colGuess Which column we think its in
- * @return {Object} The item object for the given title
- */
- findItemInResArray = function ( title, indexGuess,
colGuess ) {
- var oneLess,
- oneMore,
- i,
- j,
- resItems = res.content.columns[
colGuess ].items;
-
- indexGuess %= resItems.length;
-
- if ( resItems[ indexGuess ].title === title ) {
- return resItems[ indexGuess ];
- }
-
- oneMore = ( indexGuess + 1 ) % resItems.length;
- oneLess = indexGuess - 1 < 0 ? resItems.length
- 1 : indexGuess - 1;
- if ( resItems[ oneMore ].title === title ) {
- return resItems[ oneMore ];
- }
-
- if ( resItems[ oneLess ].title === title ) {
- return resItems[ oneLess ];
- }
-
- // Still here, check entire array.
- for ( i = 0; i < res.content.columns.length;
i++ ) {
- for ( j = 0; j < res.content.columns[ i
].items.length; j++ ) {
- if ( res.content.columns[ i
].items[ j ].title === title ) {
- return
res.content.columns[ i ].items[ j ];
- }
+ outer: for ( i = 0; i < newOrder.length; i++ ) {
+ for ( j = 0; j < newOrder[ i ].length; j++ ) {
+ if ( newOrder[ i ][ j ] === oldPosition
) {
+ newPosition = i + '-' + j;
+ break outer;
}
}
-
- // Must be missing.
- // FIXME sane error handling.
- alert( mw.msg(
'collaborationkit-list-error-editconflict' ) );
- location.reload();
- throw new Error( 'Item ' + title + ' is
missing' );
- };
- resArray = [];
- for ( i = 0; i < newOrder.length; i++ ) {
- resArray[ i ] = [];
- for ( j = 0; j < newOrder[ i ].length; j++ ) {
- resArray[ i ][ j ] =
findItemInResArray( newOrder[ i ][ j ], j, i );
- }
}
- for ( i = 0; i < resArray.length; i++ ) {
- res.content.columns[ i ].items = resArray[ i ];
+
+ resColumns = [];
+ for ( i = 0; i < res.content.columns.length; i++ ) {
+ resColumns[ i ] = res.content.columns[ i
].items;
+ }
+
+ moveFrom = oldPosition.split( '-' ); // 0 = column; 1
= item
+ moveTo = newPosition.split( '-' ); // 0 = column; 1 =
item
+
+ movingItem = resColumns[ moveFrom[ 0 ] ][ moveFrom[ 1 ]
];
+ resColumns[ moveFrom[ 0 ] ].splice( moveFrom[ 1 ], 1 );
+ resColumns[ moveTo[ 0 ] ].splice( moveTo[ 1 ], 0,
movingItem );
+
+ for ( i = 0; i < res.content.columns.length; i++ ) {
+ res.content.columns[ i ].items = resColumns[ i
];
}
res.summary = mw.msg(
'collaborationkit-list-move-summary', reorderedItem );
saveJson( res, function () {
spinner.remove();
+ renumberElements( moveFrom[ 0 ] );
+ renumberElements( moveTo[ 0 ] );
mw.notify(
mw.msg(
'collaborationkit-list-move-popup', reorderedItem ),
{
@@ -295,10 +282,11 @@
module.exports = {
getColId: getColId,
deleteItem: deleteItem,
- getListOfTitles: getListOfTitles,
+ getListOfItems: getListOfItems,
reorderList: reorderList,
getCurrentJson: getCurrentJson,
- saveJson: saveJson
+ saveJson: saveJson,
+ renumberElements: renumberElements
};
} )( jQuery, mediaWiki );
diff --git a/modules/ext.CollaborationKit.list.ui.js
b/modules/ext.CollaborationKit.list.ui.js
index 26a0574..f77ae2b 100644
--- a/modules/ext.CollaborationKit.list.ui.js
+++ b/modules/ext.CollaborationKit.list.ui.js
@@ -36,30 +36,31 @@
* Edit an existing item.
*
* @param {string} itemName The title of the item in question
+ * @param {string} itemId ID string corresponding to the column/item
index
* @param {number} colId Which column the item is in
*/
- modifyExistingItem = function ( itemName, colId ) {
+ modifyExistingItem = function ( itemName, itemId, colId ) {
LE.getCurrentJson( mw.config.get( 'wgArticleId' ), function (
res ) {
- var done = false;
+ var done, itemIdNumber, toEdit;
+ done = false;
// Member lists pretend to have two columns (active and
inactive),
// but in the source code, it's only one column. This
forces one column
// for the purposes of the JS editor.
if ( mw.config.get( 'wgCollaborationKitIsMemberList' )
) {
colId = 0;
}
- $.each( res.content.columns[ colId ].items, function (
index ) {
- if ( this.title === itemName ) {
- done = true;
- modifyItem( {
- itemTitle: this.title,
- itemImage: this.image,
- itemDescription: this.notes,
- itemIndex: index,
- itemColId: colId
- } );
- return false;
- }
+
+ itemIdNumber = parseInt( itemId.split( '-' )[ 1 ] );
+ toEdit = res.content.columns[ colId ].items[
itemIdNumber ];
+ done = true;
+ modifyItem( {
+ itemTitle: toEdit.title,
+ itemImage: toEdit.image,
+ itemDescription: toEdit.notes,
+ itemIndex: itemIdNumber,
+ itemColId: colId
} );
+
if ( !done ) {
// FIXME error handling
alert( mw.msg(
'collaborationkit-list-error-edit' ) );
@@ -273,7 +274,10 @@
label: 'edit',
framed: false
} ).on( 'click', function () {
- modifyExistingItem( item.data(
'collabkit-item-title' ), colId );
+ modifyExistingItem(
+ item.data(
'collabkit-item-title' ),
+ item.data( 'collabkit-item-id'
),
+ colId );
} );
// FIXME, the <a> might make an extra target
when tabbing
@@ -321,28 +325,36 @@
start: function ( e ) {
$( e.target )
.addClass( 'mw-ck-dragging' )
- .data( 'startTitleList',
LE.getListOfTitles( $list ) );
+ .data( 'startItemList',
LE.getListOfItems( $list ) );
},
stop: function ( e, ui ) {
- var oldListTitles, newListTitles,
$target, i, j, changed, count;
+ var oldListItems,
+ newListItems,
+ oldPosition,
+ newPosition,
+ $target,
+ i,
+ j,
+ changed,
+ count;
$target = $( e.target );
$target.removeClass( 'mw-ck-dragging' );
- oldListTitles = $target.data(
'startTitleList' );
- newListTitles = LE.getListOfTitles(
$list );
- $target.data( 'startTitleList', null );
+ oldListItems = $target.data(
'startItemList' );
+ newListItems = LE.getListOfItems( $list
);
+ $target.data( 'startItemList', null );
// FIXME better error handling
- if ( oldListTitles.length !==
newListTitles.length ) {
+ if ( oldListItems.length !==
newListItems.length ) {
throw new Error( 'We somehow
lost a column?!' );
}
changed = false;
count = 0;
- for ( i = 0; i < oldListTitles.length;
i++ ) {
- count += oldListTitles.length;
- count -= newListTitles.length;
- for ( j = 0; j < oldListTitles[
i ].length; j++ ) {
- if ( oldListTitles[ i
][ j ] !== newListTitles[ i ][ j ] ) {
+ for ( i = 0; i < oldListItems.length;
i++ ) {
+ count += oldListItems.length;
+ count -= newListItems.length;
+ for ( j = 0; j < oldListItems[
i ].length; j++ ) {
+ if ( oldListItems[ i ][
j ] !== newListItems[ i ][ j ] ) {
changed = true;
break;
}
@@ -353,7 +365,7 @@
throw new Error( 'List item has
disappeared?' );
}
if ( changed ) {
- LE.reorderList( ui.item,
newListTitles, oldListTitles );
+ LE.reorderList( ui.item,
newListItems, oldListItems );
}
}
} );
--
To view, visit https://gerrit.wikimedia.org/r/351094
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I01af89264c59f943274c422b630b1f9fb09622ca
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CollaborationKit
Gerrit-Branch: master
Gerrit-Owner: Harej <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
