Add Dueling Picklist

[kyleheddon]

I've implemented a new <DuelingPicklist /> component for my team's internal project (Salesforce employee), which I would like to contribute.

Proposed propTypes:

static propTypes = {
    /**
     * allows the user to reorder the second listbox of options
     */
    allowReordering: PropTypes.bool,

    /**
     * **Assistive text for accessibility**
     * This object is merged with the default props object on every render.
     * * `optionDragLabel`: Instructions on how to drag and drop with a keyboard.
     * * `moveSelectionUp`: Used by "up" reordering button.
     * * `moveSelectionDown`: Used by "down" reordering button.
     * * `moveSelectionToSecondCategory`: Used by "right" button, which moves selected items to selection.
     * * `moveSelectionToFirstCategory`: Used by "left" button, which removes selected items from selection.
     * * `tooltipIcon`: Used by tooltip, if a tooltip is being used.
     * * `itemLocked`: Used to label locked items.
     * * `itemsMovedToSelection`: Used in Aria Live area to inform user that items were moved to selection.
     * * `itemsRemovedFromSelection`: Used in Aria Live area to inform user that items were removed from selection..
     * * `itemsReorderedInSelection`: Used in Aria Live area to inform user that items were reordered in selection.
     */ 
    assistiveText: PropTypes.shape({
        optionDragLabel: PropTypes.string,
        moveSelectionUp: PropTypes.string,
        moveSelectionDown: PropTypes.string,
        moveSelectionToSecondCategory: PropTypes.string,
        moveSelectionToFirstCategory: PropTypes.string,
        tooltipIcon: PropTypes.string,
        itemLocked: PropTypes.string,
        itemsMovedToSelection: PropTypes.string,
        itemsRemovedFromSelection: PropTypes.string,
        itemsReorderedInSelection: PropTypes.string,
    }),

    /**
     * When true, the height of both listboxes will be the smallest height needed to show all items without having to scroll.
     */ 
    automaticHeightMinimization: PropTypes.bool,

    /**
     * When true, all interactions are disabled.
     */
    disabled: PropTypes.bool,
    
    /**
     * Event Callbacks
     * * `onChange`: Called when items are added or removed from `selection`
     */
    events: PropTypes.shape({
        onChange: PropTypes.func.isRequired,
    }).isRequired,

    /**
     * When true, shows a tooltip.
     */
    hasTooltip: PropTypes.bool,

    /**
     * Manually sets the height of both listboxes.
     */
    height: PropTypes.string,

    /**
     * Element ids (used for accessibility). If not provided, ids will be generated with shortid.
     * * `picklistGroupLabel`: id for labeling the `<DuelingPicklist />` component.
     * * `dragLiveRegion`: id for Aria Live element.
     * * `optionDragLabel`: id for describing how to use keyboard interactions.
     * * `firstCategoryLabel`: id for options listbox.
     * * `secondCategoryLabel`: id for selection listbox.
     */
    ids: PropTypes.shape({
        picklistGroupLabel: PropTypes.string,
        dragLiveRegion: PropTypes.string,
        optionDragLabel: PropTypes.string,
        firstCategoryLabel: PropTypes.string,
        secondCategoryLabel: PropTypes.string,
    }),

    /**
     * Labels
     * * `heading`: Heading for component.
     * * `firstCategory`: Label for options.
     * * `secondCategory`: Label for selection.
     * * `tooltip`: Label for tooltip.
     * * `selectedItems`: Labels selected items. Used in View Mode.
     */
    labels: PropTypes.shape({
        heading: PropTypes.string,
        firstCategory: PropTypes.string,
        secondCategory: PropTypes.string,
        tooltip: PropTypes.string,
        selectedItems: PropTypes.string,
    }),

    /**
     * Items in the first listbox
     * * `label`: Item label.
     * * `id`: Unique id for the item.
     * * `locked`: When true, a lock icon renders on the item, and the item cannot be moved to or from the selection.
     */
    options: PropTypes.arrayOf(PropTypes.shape({
        label: PropTypes.string.isRequired,
        id: PropTypes.string.isRequired,
        locked: PropTypes.bool,
    })).isRequired,

    /**
     * When true, the component will be render with responsive css classes applied. Any items longer than the space available will truncate with ellipses.
     */
    responsive: PropTypes.bool,

    /**
     * When true, a red asterisk will render, visually marking the item as required.
     */
    required: PropTypes.bool,

    /**
     * Items in the second listbox
     * * `label`: Item label.
     * * `id`: Unique id for the item.
     * * `locked`: When true, a lock icon renders on the item, and the item cannot be moved to or from the selection.
     */
    selection: PropTypes.arrayOf(PropTypes.shape({
        label: PropTypes.string.isRequired,
        id: PropTypes.string.isRequired,
        locked: PropTypes.bool,
    })).isRequired,

    /**
     * When true, the component will render in View Mode, which only shows the items in the selection.
     */
    viewMode: PropTypes.bool,
}

[welcome]

Thanks for opening your first issue! 👋
If you have found this library helpful, please star it. A maintainer will try to respond within 7 days. If you haven’t heard anything by then, please bump this thread.

[interactivellama]

Awesome! Excellence consistency with the existing prop comment format.

• With the tooltip, I'd like to align with the existing fieldLevelHelpTooltip which takes a Tooltip component and should remove some of the assistiveText and labels keys and allow complete control of tooltip by consumer dev.
• Please update label.heading to labels.label for consistency as a form label unless you think that is confusing, since there are three labels.
• I'd like to be a little more semantic with the first and second category. options and selected based on your other props are good names. firstCategory to options and secondCategory to selected. Also in ids object
• Booleans like allowReordering are typically something like isReorderable in this library.
• Let's be more specific with height, so that folks don't think it's the container heightOfListbox or listboxHeight is fine.
• What do you think of something with the term viewMode in it for labels.selectedItems?
• DSR usually does id with internal hard coded suffixes for the child nodes. I'm open to ids, but this will be the first component with that API. I can see how it would make testing easier by starting with variables to began with.

[kyleheddon]

Thanks!

With the tooltip, I'd like to align with the existing fieldLevelHelpTooltip which takes a Tooltip component and should remove some of the assistiveText and labels keys and allow complete control of tooltip by consumer dev.

👍It does make a lot of sense to let the consumer pass in the tooptip.

Please update label.heading to labels.label for consistency as a form label unless you think that is confusing, since there are three labels.

It does seem a bit confusing to have labels.label given that there are 3. Are there any other components in project that have multiple, with one being the "main" label?

I'd like to be a little more semantic with the first and second category. options and selected based on your other props are good names. firstCategory to options and secondCategory to selected. Also in ids object.

👍 Makes sense. "category" is an artifact from the original impl - I later changed to options and selection. Should it be selection, rather than selected? "Selected" is a term used by SLDS to refer to the items that are selected (and highlighted) in either listbox.

Booleans like allowReordering are typically something like isReorderable in this library.

👍

Let's be more specific with height, so that folks don't think it's the container heightOfListbox or listboxHeight is fine.

👍

What do you think of something with the term viewMode in it for labels.selectedItems?

👍 I think this makes a lot of sense. labels.selectedItems has proven to be confusing in our development/testing.

DSR usually does id with internal hard coded suffixes for the child nodes. I'm open to ids, but this will be the first component with that API. I can see how it would make testing easier by starting with variables to began with.

Would the alternative be having props like picklistGroupLabelId, firstCategoryLabelId, secondCategoryLabelId, etc?

[interactivellama]

It does seem a bit confusing to have labels.label given that there are 3. Are there any other components in project that have multiple, with one being the "main" label?

It's describing the same as picklistGroupLabel, so this might be good:

`labels.group`: A `DuelingPicklist` should have a group label, similar to using a `fieldset` HTML element.

Should it be selection, rather than selected?

It's noun vs adjective. active or focused is often used in this library for user UI "selection". The ARIA spec uses the term selected for UI focus and data selection for single selection, but not for multiple selection, so the terms overlap from time to time. Here are some of combobox's props using selected onRequestRemoveSelectedOption
multipleOptionsSelected

Would the alternative be having props like picklistGroupLabelId, firstCategoryLabelId, secondCategoryLabelId, etc?

Oh, sorry no. It's typically be internally suffixed, kabob case. such as ${this.getId()}-selected-listbox, but if it's easier to provide all the ids as the consuming developer, I'm open to it. Do you use these ids in other part of your code? Are these id's used in tests, etc.? If so, allowing all custom id's is probably a good idea and allows the most control for you all (less DOM inspecting for hidden strings 😉 )

---

Also, I probably should document this somewhere, but labels are often propType of: PropTypes.oneOfType([PropTypes.node, PropTypes.string]), because consuming devs might want to use italics and a span for styling when the "text" is rendered, however it's only a string if HTML tags would break it--for instance title or an input placeholder.

Looking forward to seeing the PR!

[kyleheddon]

The ids are only used for accessibility (internally used by the component). My current impl does not actually take in ids as props, rather it creates them internally:

this.elementIds = {
	picklistGroupLabel: `picklist-group-label-${shortid.generate()}`,
	dragLiveRegion: `drag-live-region-${shortid.generate()}`,
	optionDragLabel: `option-drag-label-${shortid.generate()}`,
	firstCategoryLabel: `first-category-label-${shortid.generate()}`,
	secondCategoryLabel: `second-category-label-${shortid.generate()}`,
}

I saw other components take in id, so I thought I would allow the consumer to pass them in for the sake of consistency in the proposed props.

That being said, I can't think of a good reason (from a best practices perspective) for allowing custom ids to be passed in. Taking in ids as props and doing it like ${this.getId()}-selected-listbox seems like a reasonable compromise.

[interactivellama]

Sounds good. I think we align. Tell me if there's anything else I can do for you.

[tanhengyeow]

@kyleheddon @interactivellama Would be more than happy to move this forward and I've started making changes from the review :)

[kyleheddon]

@tanhengyeow I'm open you changes from you. I've been putting out some fires and will not likely be able to make changes to this for at least a couple weeks

[pmartin35]

@kyleheddon @interactivellama Hey guys, would it be possible to merge this component in?