jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/354618 )
Change subject: Assign column-agnostic UID to individual list entries
......................................................................
Assign column-agnostic UID to individual list entries
Unlike the other ID assignment mechanism, this is based on the item's
position in the JSON's native representation. This allows list items
to be edited regardless of how they appear in the rendered HTML.
Bug: T165672
Change-Id: I37da1639e2e70d1b51ba090444d2a5e73be94613
---
M includes/content/CollaborationListContent.php
M modules/ext.CollaborationKit.list.edit.js
M modules/ext.CollaborationKit.list.ui.js
3 files changed, 221 insertions(+), 150 deletions(-)
Approvals:
Brian Wolff: Looks good to me, approved
Harej: Looks good to me, but someone else must approve
jenkins-bot: Verified
diff --git a/includes/content/CollaborationListContent.php
b/includes/content/CollaborationListContent.php
index 47c68ff..8027394 100644
--- a/includes/content/CollaborationListContent.php
+++ b/includes/content/CollaborationListContent.php
@@ -300,10 +300,19 @@
return $text;
}
+ $columns = $this->columns;
+
+ // Assign a UID to each list entry.
+ $uidCounter = 0;
+ foreach ( $columns as $colId => $column ) {
+ foreach ( $column->items as $rowId => $row ) {
+ $columns[$colId]->items[$rowId]->uid =
$uidCounter;
+ $uidCounter++;
+ }
+ }
+
if ( $this->displaymode === 'members' && count( $this->columns
) === 1 ) {
- $columns = $this->sortUsersIntoColumns(
$this->columns[0] );
- } else {
- $columns = $this->columns;
+ $columns = $this->sortUsersIntoColumns( $columns[0] );
}
$columns = $this->filterColumns( $columns, $options['columns']
);
@@ -335,106 +344,111 @@
if ( count( $column->items ) === 0 ) {
$text .= "\n<div class=\"mw-ck-list-item\">";
$text .=
"{{mediawiki:collaborationkit-list-emptycolumn}}</div>\n";
- $text .= "</div>\n";
- continue;
- }
+ } else {
+ $curItem = 0;
- $curItem = 0;
+ $sortedItems = $column->items;
+ $this->sortList( $sortedItems,
$options['defaultSort'] );
- $sortedItems = $column->items;
- $this->sortList( $sortedItems, $options['defaultSort']
);
-
- $itemCounter = 0;
- foreach ( $sortedItems as $item ) {
- if ( $offset !== 0 ) {
- $offset--;
- continue;
- }
- $curItem++;
- if ( $maxItems !== false && $maxItems <
$curItem ) {
- break;
- }
-
- $itemTags = isset( $item->tags ) ? $item->tags
: [];
- if ( !$this->matchesTag( $options['tags'],
$itemTags ) ) {
- continue;
- }
-
- $titleForItem = null;
- if ( !isset( $item->link ) ) {
- $titleForItem = Title::newFromText(
$item->title );
- } elseif ( $item->link !== false ) {
- $titleForItem = Title::newFromText(
$item->link );
- }
- $text .= Html::openElement( 'div', [
- 'style' => "min-height:{$iconWidth}px",
- 'class' => 'mw-ck-list-item',
- '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(
- $item->image,
- $this->displaymode,
- $titleForItem,
- $iconWidth
- );
- } else {
- // Use fallback mechanisms
- $text .= static::generateImage(
- null,
- $this->displaymode,
- $titleForItem,
- $iconWidth
- );
+ $itemCounter = 0;
+ foreach ( $sortedItems as $item ) {
+ if ( $offset !== 0 ) {
+ $offset--;
+ continue;
}
- }
+ $curItem++;
+ if ( $maxItems !== false && $maxItems <
$curItem ) {
+ break;
+ }
- $text .= '<div class="mw-ck-list-container">';
- // Question: Arguably it would be more
semantically correct to
- // use an <Hn> element for this. Would that be
better? Unclear.
- $text .= '<div class="mw-ck-list-title">';
- if ( $titleForItem ) {
- if ( $this->displaymode == 'members'
- && !isset( $item->link )
- && $titleForItem->inNamespace(
NS_USER )
+ $itemTags = isset( $item->tags ) ?
$item->tags : [];
+ if ( !$this->matchesTag(
$options['tags'], $itemTags ) ) {
+ continue;
+ }
+
+ $titleForItem = null;
+ if ( !isset( $item->link ) ) {
+ $titleForItem =
Title::newFromText( $item->title );
+ } elseif ( $item->link !== false ) {
+ $titleForItem =
Title::newFromText( $item->link );
+ }
+ $text .= Html::openElement( 'div', [
+ 'style' =>
"min-height:{$iconWidth}px",
+ 'class' => 'mw-ck-list-item',
+ 'data-collabkit-item-title' =>
$item->title,
+ 'data-collabkit-item-id' =>
$colId . '-' . $itemCounter,
+ 'data-collabkit-item-uid' =>
$item->uid
+ ] );
+ $itemCounter++;
+ if ( $options['mode'] !== 'no-img' ) {
+ if ( isset( $item->image ) ) {
+ $text .=
static::generateImage(
+ $item->image,
+
$this->displaymode,
+ $titleForItem,
+ $iconWidth
+ );
+ } else {
+ // Use fallback
mechanisms
+ $text .=
static::generateImage(
+ null,
+
$this->displaymode,
+ $titleForItem,
+ $iconWidth
+ );
+ }
+ }
+
+ $text .= '<div
class="mw-ck-list-container">';
+ // Question: Arguably it would be more
semantically correct to
+ // use an <Hn> element for this. Would
that be better? Unclear.
+ $text .= '<div
class="mw-ck-list-title">';
+ if ( $titleForItem ) {
+ if ( $this->displaymode ==
'members'
+ && !isset( $item->link )
+ &&
$titleForItem->inNamespace( NS_USER )
+ ) {
+ $titleText =
$titleForItem->getText();
+ } else {
+ $titleText =
$item->title;
+ }
+ $text .= '[[:' .
$titleForItem->getPrefixedDBkey() . '|'
+ . wfEscapeWikiText(
$titleText ) . ']]';
+ } else {
+ $text .= wfEscapeWikiText(
$item->title );
+ }
+ $text .= "</div>\n";
+ $text .= '<div
class="mw-ck-list-notes">' . "\n";
+ if ( isset( $item->notes ) &&
is_string( $item->notes ) ) {
+ $text .= $item->notes . "\n";
+ }
+
+ if ( isset( $item->tags ) && is_array(
$item->tags )
+ && count( $item->tags )
) {
- $titleText =
$titleForItem->getText();
- } else {
- $titleText = $item->title;
+ $text .= "\n<div
class='toccolours mw-ck-list-tags'>" .
+ wfMessage(
'collaborationkit-list-taglist' )
+ ->inLanguage(
$lang )
+ ->params(
+
$lang->commaList(
+
array_map( 'wfEscapeWikiText', $item->tags )
+ )
+ )->numParams(
count( $item->tags ) )
+ ->text() .
+ "</div>\n";
}
- $text .= '[[:' .
$titleForItem->getPrefixedDBkey() . '|'
- . wfEscapeWikiText( $titleText
) . ']]';
- } else {
- $text .= wfEscapeWikiText(
$item->title );
+ $text .= '</div></div></div>' . "\n";
}
- $text .= "</div>\n";
- $text .= '<div class="mw-ck-list-notes">' .
"\n";
- if ( isset( $item->notes ) && is_string(
$item->notes ) ) {
- $text .= $item->notes . "\n";
- }
-
- if ( isset( $item->tags ) && is_array(
$item->tags )
- && count( $item->tags )
- ) {
- $text .= "\n<div class='toccolours
mw-ck-list-tags'>" .
- wfMessage(
'collaborationkit-list-taglist' )
- ->inLanguage( $lang )
- ->params(
-
$lang->commaList(
-
array_map( 'wfEscapeWikiText', $item->tags )
- )
- )->numParams( count(
$item->tags ) )
- ->text() .
- "</div>\n";
- }
- $text .= '</div></div></div>' . "\n";
+ }
+ if ( $this->displaymode != 'members' ) {
+ $text .= "\n<div
class=\"mw-ck-list-additem-container\"></div>";
}
$text .= "\n</div>";
}
$text .= "\n</div>";
+ if ( $this->displaymode == 'members' ) {
+ $text .= "\n<div
class=\"mw-ck-list-additem-container\"></div>";
+ }
return $text;
}
@@ -1117,7 +1131,7 @@
* Filter users into active and inactive.
*
* @note The results of this function get stored in parser cache.
- * @param array $userList Array of user dbkeys => stdClass
+ * @param array $userList Array of usernames => stdClass
* @return array [ 'active' => [..], 'inactive' => '[..]' ]
*/
private function filterActiveUsers( $userList ) {
diff --git a/modules/ext.CollaborationKit.list.edit.js
b/modules/ext.CollaborationKit.list.edit.js
index 75400fe..85e8c0d 100644
--- a/modules/ext.CollaborationKit.list.edit.js
+++ b/modules/ext.CollaborationKit.list.edit.js
@@ -46,6 +46,7 @@
spinner,
title = $item.data( 'collabkit-item-title' ),
itemId = $item.data( 'collabkit-item-id' ),
+ uid = $item.data( 'collabkit-item-uid' ),
colId = getColId( $item );
if ( mw.config.get( 'wgCollaborationKitIsMemberList' ) ) {
@@ -65,7 +66,7 @@
var newItems = [];
oldItems = res.content.columns[ colId ].items;
for ( i = 0; i < oldItems.length; i++ ) {
- if ( colId + '-' + i === itemId ) {
+ if ( oldItems[ i ].uid === uid ) {
continue;
}
newItems[ newItems.length ] = oldItems[ i ];
@@ -97,14 +98,18 @@
* @return {Array} 2D array of all items in all columns.
*/
getListOfItems = function ( $elm ) {
- var list = [];
+ var list = [],
+ relevantItem;
$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-id' );
+ relevantItem = $( this ).data(
'collabkit-item-id' );
+ if ( relevantItem ) {
+ list[ colId ][ list[ colId ].length ] =
relevantItem;
+ }
} );
} );
return list;
@@ -217,7 +222,10 @@
* @param {Object} callback
*/
getCurrentJson = function ( pageId, callback ) {
- var api = new mw.Api();
+ var api = new mw.Api(),
+ i,
+ j,
+ uidCounter = 0;
api.get( {
action: 'query',
@@ -247,6 +255,15 @@
res.pageid = pageId;
res.timestamp = rev.timestamp;
res.content = JSON.parse( rev[ '*' ] );
+
+ // Assigning UID to each list entry
+ for ( i = 0; i < res.content.columns.length; i++ ) {
+ for ( j = 0; j < res.content.columns[ i
].items.length; j++ ) {
+ res.content.columns[ i ].items[ j ].uid
= uidCounter;
+ uidCounter++;
+ }
+ }
+
callback( res );
} ).fail(
function () { alert( mw.msg(
'collaborationkit-list-error-generic' ) ); }
@@ -260,7 +277,16 @@
* @param {Object} callback
*/
saveJson = function ( params, callback ) {
- var api = new mw.Api();
+ var api = new mw.Api(),
+ i,
+ j;
+
+ // Strip out UID; we don't want to save it.
+ for ( i = 0; i < params.content.columns.length; i++ ) {
+ for ( j = 0; j < params.content.columns[ i
].items.length; j++ ) {
+ delete params.content.columns[ i ].items[ j
].uid;
+ }
+ }
// This will explode if we hit a captcha
api.postWithEditToken( {
diff --git a/modules/ext.CollaborationKit.list.ui.js
b/modules/ext.CollaborationKit.list.ui.js
index 0c3dbc3..7560ad4 100644
--- a/modules/ext.CollaborationKit.list.ui.js
+++ b/modules/ext.CollaborationKit.list.ui.js
@@ -4,9 +4,37 @@
( function ( $, mw, OO ) {
'use strict';
- var addItem, modifyItem, modifyExistingItem, LE;
+ var getItemFromUid, addItem, modifyItem, modifyExistingItem, LE;
LE = require( 'ext.CollaborationKit.list.edit' );
+
+ /**
+ * Finds item object based on a UID.
+ *
+ * @param {number} uid The relevant unique ID
+ * @param {Object} columns The columns object from the content object
+ * @return {Object} An object with keys relevantItem, relevantColumn,
relevantRow
+ */
+ getItemFromUid = function ( uid, columns ) {
+ var i, j, relevantItem, relevantColumn, relevantRow;
+
+ outer: for ( i = 0; i < columns.length; i++ ) {
+ for ( j = 0; j < columns[ i ].items.length; j++ ) {
+ if ( columns[ i ].items[ j ].uid === uid ) {
+ relevantItem = columns[ i ].items[ j ];
+ relevantColumn = i;
+ relevantRow = j;
+ break outer;
+ }
+ }
+ }
+
+ return {
+ relevantItem: relevantItem,
+ relevantColumn: relevantColumn,
+ relevantRow: relevantRow
+ };
+ };
/**
* Adds a new item to a list
@@ -20,7 +48,8 @@
/**
* Opens a window to manage list item modification
*
- * @param {Object} itemToEdit The name of the title to modify, or false
to add new.
+ * @param {Object} itemToEdit Data concerning the item to edit/add. At
+ * minimum you need an itemColId attribute with the column ID.
*/
modifyItem = function ( itemToEdit ) {
var dialog, windowManager;
@@ -36,29 +65,22 @@
* 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
+ * @param {string} uid Unique identifier based on order in native JSON
representation
*/
- modifyExistingItem = function ( itemName, itemId, colId ) {
+ modifyExistingItem = function ( itemName, uid ) {
LE.getCurrentJson( mw.config.get( 'wgArticleId' ), function (
res ) {
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;
- }
- itemIdNumber = parseInt( itemId.split( '-' )[ 1 ] );
- toEdit = res.content.columns[ colId ].items[
itemIdNumber ];
+ toEdit = getItemFromUid( uid, res.content.columns );
+
done = true;
modifyItem( {
- itemTitle: toEdit.title,
- itemImage: toEdit.image,
- itemDescription: toEdit.notes,
- itemIndex: itemIdNumber,
- itemColId: colId
+ itemTitle: toEdit.relevantItem.title,
+ itemImage: toEdit.relevantItem.image,
+ itemDescription: toEdit.relevantItem.notes,
+ itemColId: toEdit.relevantColumn,
+ itemUid: uid
} );
if ( !done ) {
@@ -82,7 +104,7 @@
this.itemTitle = config.itemTitle;
this.itemDescription = config.itemDescription;
this.itemImage = config.itemImage;
- this.itemIndex = config.itemIndex;
+ this.itemUid = config.itemUid;
}
if ( config.itemColId ) {
this.itemColId = config.itemColId;
@@ -109,7 +131,7 @@
* @param {Object} itemInfo info from json
*/
NewItemDialog.prototype.initialize = function ( itemInfo ) {
- var description, itemTitleObj;
+ var description, itemTitleObj, i, j;
NewItemDialog.parent.prototype.initialize.apply( this,
arguments );
this.panel1 = new OO.ui.PanelLayout( { padded: true, expanded:
false } );
@@ -200,27 +222,38 @@
}
LE.getCurrentJson( mw.config.get( 'wgArticleId' ), function (
res ) {
- var index, itemToAdd = {
- title: title,
- notes: notes
- };
+ var index,
+ itemToAdd = {
+ title: title,
+ notes: notes
+ },
+ relevantItem,
+ relevantRow;
if ( file ) {
itemToAdd.image = file;
}
- if ( dialog.itemIndex !== undefined ) {
- if ( res.content.columns[ dialog.itemColId
].items <= dialog.itemIndex ||
- res.content.columns[ dialog.itemColId
].items[ dialog.itemIndex ].title !== dialog.itemTitle
- ) {
+ if ( dialog.itemUid !== undefined ) {
+ // We are editing an existing item
+
+ relevantItem = getItemFromUid( dialog.itemUid,
res.content.columns );
+
+ relevantRow = relevantItem.relevantRow;
+ relevantItem = relevantItem.relevantItem;
+
+ // Edit conflict detection
+ if ( relevantItem.title !== dialog.itemTitle ) {
alert( mw.msg(
'collaborationkit-list-error-editconflict' ) );
location.reload();
// fixme proper handling.
throw new Error( 'edit conflict' );
}
- index = dialog.itemIndex;
+
+ index = relevantRow;
} else {
index = res.content.columns[ dialog.itemColId
].items.length;
}
+
res.content.columns[ dialog.itemColId ].items[ index ]
= itemToAdd;
res.summary = mw.msg(
'collaborationkit-list-add-summary', title );
LE.saveJson( res, function () {
@@ -240,11 +273,7 @@
}
$list = $( '.mw-ck-list' );
- if ( mw.config.get( 'wgCollaborationKitIsMemberList' ) ) {
- column = $(
'.mw-ck-list-column[data-collabkit-column-id=0] .mw-ck-list-item:last-child' );
- } else {
- column = $( '.mw-ck-list-item:last-child' );
- }
+
$list.find( '.mw-ck-list-item' ).each( function () {
var deleteButton,
moveButton,
@@ -278,8 +307,7 @@
} ).on( 'click', function () {
modifyExistingItem(
item.data(
'collabkit-item-title' ),
- item.data( 'collabkit-item-id'
),
- colId );
+ item.data( 'collabkit-item-uid'
) );
} );
// FIXME, the <a> might make an extra target
when tabbing
@@ -376,22 +404,25 @@
buttonMsg = mw.config.get( 'wgCollaborationKitIsMemberList' ) ?
'collaborationkit-list-add-user' :
'collaborationkit-list-add';
- column.after(
- $( '<div></div>' )
- // FIXME There is probably a way to add the
class without
- // extra div.
- .addClass( 'mw-ck-list-additem' )
- .append(
- new OO.ui.ButtonWidget( {
- label: mw.msg( buttonMsg ),
- icon: 'add',
- flags: 'constructive'
- } ).on( 'click', function () {
- addItem( $( event.target
).closest( '.mw-ck-list-column' ).data( 'collabkit-column-id' ) );
- } )
- .$element
- )
- );
+ $( '.mw-ck-list-additem-container' ).each( function () {
+ var $addButton = $( this );
+ $addButton.append(
+ $( '<div></div>' )
+ // FIXME There is probably a way to add
the class without
+ // extra div.
+ .addClass( 'mw-ck-list-additem' )
+ .append(
+ new OO.ui.ButtonWidget( {
+ label: mw.msg(
buttonMsg ),
+ icon: 'add',
+ flags: 'constructive'
+ } ).on( 'click', function (
event ) {
+ addItem(
$addButton.closest( '.mw-ck-list-column' ).data( 'collabkit-column-id' ) );
+ } )
+ .$element
+ )
+ );
+ } );
} );
} )( jQuery, mediaWiki, OO );
--
To view, visit https://gerrit.wikimedia.org/r/354618
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I37da1639e2e70d1b51ba090444d2a5e73be94613
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/CollaborationKit
Gerrit-Branch: master
Gerrit-Owner: Harej <jamesmh...@gmail.com>
Gerrit-Reviewer: Brian Wolff <bawolff...@gmail.com>
Gerrit-Reviewer: Harej <jamesmh...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
