I'm building a GroupPicker class that essentially manages an interface that allows the user to move items between two columns "assigned" vs "unassigned". The items within the columns have a hierarchy of parents and children, so if a child is assigned, so is the parent.
What I'm doing could be completely fine, but it seems like a code smell to me to have the HTML coupled into the class itself. The class needs to be flexible because sometimes the items are not tiered, sometimes we don't want to be able to remove already assigned items, etc., and having it coupled with the HTML seems to make it pretty rigid.
What is the best practice when dealing with a class that's sole purpose is to manage the state of the HTML and the values of the inputs?
<!-- The HTML Structure -->
<div data-grouppicker="grouppage">
<div>
<div>Available Contact Groups</div>
<div>
<ul id="unassigned">
<?php foreach ( $DropDownItems as $Item ) { ?>
<?php [$ID, $Description, $TreeLevel, $ParentID, $HasChildren, $ChildrenIDs, $Count] = explode( ',', $Item ); ?>
<?php $IsAssignedParent = ( ! in_array( $ID, $UnassignedIDArray ) && $HasChildren ); ?>
<?php if ( in_array( $ID, $UnassignedIDArray ) ) { ?>
<li
id="list<?php echo $ID; ?>"
<?php echo $IsAssignedParent ? 'class="disabledPickListItem"' : ''; ?>
data-id="<?php echo $ID; ?>"
data-counter="<?php echo $Count; ?>"
data-parent="<?php echo $ParentID; ?>"
data-children="<?php echo $ChildrenIDs; ?>"
data-treelevel="<?php echo $TreeLevel; ?>"
data-disabled="<?php echo $IsAssignedParent ? 'true' : 'false'; ?>"
>
<div><?php echo $Description; ?></div>
<img src="images/add_icon.png" data-assign="<?php echo $ID; ?>" />
</li>
<?php } ?>
<?php } ?>
</ul>
</div>
</div>
<div>
<div>Assigned Contact Groups</div>
<div>
<ul id="assigned">
<?php foreach ( $DropDownItems as $Item ) { ?>
<?php [$ID, $Description, $TreeLevel, $ParentID, $HasChildren, $ChildrenIDs, $Count] = explode( ',', $Item ); ?>
<?php if ( in_array( $ID, $AssignedIDArray ) ) { ?>
<li
id="list<?php echo $ID; ?>"
data-id="<?php echo $ID; ?>"
data-counter="<?php echo $Count; ?>"
data-parent="<?php echo $ParentID; ?>"
data-children="<?php echo $ChildrenIDs; ?>"
data-treelevel="<?php echo $TreeLevel; ?>"
data-disabled="false"
>
<div><?php echo $Description; ?></div>
<img src="images/remove_icon.png" data-unassign="<?php echo $ID; ?>" />
</li>
<?php } ?>
<?php } ?>
</ul>
</div>
</div>
<input type="hidden" id="assignedValues" name="assignedValues" value="<?php echo implode(',', $AssignedIDArray); ?>" autocomplete="off" />
<input type="hidden" id="unassignedValues" name="unassignedValues" value="<?php echo implode(',', $UnassignedIDArray); ?>" autocomplete="off" />
</div>
<script>var gp = new GroupPicker(document.querySelector('[data-grouppicker="grouppage"]'));</script>
Here is the start of the GroupPicker class. The constructor, _init() and assign:
// GroupPicker.js
function GroupPicker( element ) {
var self = this;
self.el = element;
self.elUnassignedInput = self.el.querySelector( 'input#unassignedValues' );
self.elAssignedInput = self.el.querySelector( 'input#assignedValues' );
self._init();
}
GroupPicker.prototype._init = function() {
var self = this;
self.el.addEventListener( 'click', function( event ) {
if ( event.target.dataset.assign ) {
var elItem = self.el.querySelector( 'ul#unassigned li[data-id="' + event.target.dataset.assign + '"]' );
self.assign( elItem );
} else if ( event.target.dataset.unassign ) {
var elItem = self.el.querySelector( 'ul#assigned li[data-id="' + event.target.dataset.unassign + '"]' );
self.unassign( elItem );
}
});
var disabledUnassigned = [].slice.call( self.el.querySelectorAll( 'ul#unassigned li[data-disabled="true"]' ) );
disabledUnassigned.forEach( function( elItem ) {
self._checkDescendants( elItem );
});
};
GroupPicker.prototype.assign = function( elItem ) {
var self = this;
var itemId = elItem.dataset.id;
self.unassigned = [].slice.call( self.el.querySelectorAll( 'ul#unassigned li' ) );
self.assigned = [].slice.call( self.el.querySelectorAll( 'ul#assigned li' ) );
self._addToAssignedInput( itemId );
self._removeFromUnassignedInput( itemId );
self._cloneToAssignedList( elItem );
self._checkDescendants( elItem );
self._handleParent( elItem );
};
I think the best example of my problem is in the method _deleteParent:
GroupPicker.prototype._deleteParent = function( elParent ) {
var self = this;
self.unassigned = [].slice.call( self.el.querySelectorAll( 'ul#unassigned li' ) );
var elParentDescendants = elParent.dataset.children.split( '|' );
var hasUnassignedDescendants = self.unassigned.some( function( elUnassignedItem ) {
return elParentDescendants.indexOf( elUnassignedItem.dataset.id ) !== -1 && elUnassignedItem.dataset.disabled == 'false';
});
var elItemHasParent = Number( elParent.dataset.parent );
var elItemParent = self.el.querySelector( 'ul#unassigned li[data-id="' + elParent.dataset.parent + '"]' );
if ( ! hasUnassignedDescendants ) {
elParent.parentNode.removeChild( elParent );
}
if ( elItemHasParent && elItemParent ) {
self._deleteParent( self.el.querySelector( 'ul#unassigned li[data-id="' + elParent.dataset.parent + '"]' ) );
}
};
When the last descendant of an item is assigned, the disabled ancestors are removed. You can see examples of assumptions made about the HTML structure that make me inherently nervous like: var elItemParent = self.el.querySelector( 'ul#unassigned li[data-id="' + elParent.dataset.parent + '"]' );.
If you had to write a class that had similar responsibilities, how would you go about writing it? Thanks for anything you can teach me--I'd love good resources about this as well. I wasn't able to find anything from googling about it.
EDIT: Even though I'm providing an example here, I think the root of my question doesn't require a code review per se. It boils down to a question of how we write classes that manage the state of a DOM and its data and if the methods on the classes modifying the state directly is a smart thing to do, especially when making assumptions about its structure when it doesn't have control over that aspect.
