Skip to content

1709 list item structure #1710

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

1709 list item structure #1710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2dfc33e
move padding to content wrapper
tomdye Mar 26, 2021
faa62e9
add padding property to list item
tomdye Mar 26, 2021
9ee7fe2
fix tests
tomdye Mar 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/examples/src/config.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ import DividedList from './widgets/list/Dividers';
import ControlledList from './widgets/list/Controlled';
import ItemRenderer from './widgets/list/ItemRenderer';
import ListItemRenderer from './widgets/list/ListItemRenderer';
import NoPadding from './widgets/list/NoPadding';
import FetchedResource from './widgets/list/FetchedResource';
import DisabledList from './widgets/list/Disabled';
import DraggableList from './widgets/list/Draggable';
Expand Down Expand Up @@ -1077,6 +1078,11 @@ export const config = {
module: ListItemRenderer,
title: 'ListItem Renderer'
},
{
filename: 'NoPadding',
module: NoPadding,
title: 'No Padding'
},
{
description:
'This example shows the menu used as a Listbox. This allows for a selection to be made and persisted. Useful for user selections and within selects / typeahead etc.',
Expand Down
39 changes: 39 additions & 0 deletions src/examples/src/widgets/list/NoPadding.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { create, tsx } from '@dojo/framework/core/vdom';
import List, { ListItem } from '@dojo/widgets/list';
import states from './states';
import icache from '@dojo/framework/core/middleware/icache';
import Example from '../../Example';
import {
createResourceTemplate,
createResourceMiddleware
} from '@dojo/framework/core/middleware/resources';

const resource = createResourceMiddleware();
const factory = create({ icache, resource });
const template = createResourceTemplate<typeof states[0]>('value');

export default factory(function NoPadding({ id, middleware: { icache, resource } }) {
return (
<Example>
<List
resource={resource({
template: template({ id, data: states }),
transform: { value: 'value', label: 'value' }
})}
onValue={(value) => {
icache.set('value', value);
}}
itemsInView={8}
>
{({ value, label }, props) => (
<ListItem {...props} padding="none">
{{
primary: label
}}
</ListItem>
)}
</List>
<p>{`Clicked On: ${JSON.stringify(icache.getOrSet('value', ''))}`}</p>
</Example>
);
});
65 changes: 46 additions & 19 deletions src/list/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ export interface ListItemProperties {
onDrop?: (event: DragEvent) => void;
/** Determines if this item is visually collapsed during DnD */
collapsed?: boolean;
/** Specifies if the list item should be padded, defaults to small */
padding?: 'none' | 'small' | 'medium';
}

export interface ListItemChildren {
Expand All @@ -116,7 +118,7 @@ export interface ListItemChildren {

const listItemFactory = create({ theme })
.properties<ListItemProperties>()
.children<ListItemChildren | RenderResult | RenderResult[]>();
.children<ListItemChildren | RenderResult>();

export const ListItem = listItemFactory(function ListItem({
properties,
Expand All @@ -140,7 +142,8 @@ export const ListItem = listItemFactory(function ListItem({
movedDown,
collapsed,
theme: themeProp,
variant
variant,
padding = 'medium'
} = properties();

const themedCss = theme.classes(listItemCss);
Expand All @@ -153,10 +156,44 @@ export const ListItem = listItemFactory(function ListItem({
!disabled && !active && onRequestActive();
}

const [firstChild, ...otherChildren] = children();
const { leading = undefined, primary, trailing = undefined } = isRenderResult(firstChild)
? { primary: [firstChild, ...otherChildren] }
: firstChild;
const [firstChild] = children();
let listContents: RenderResult;

if (isRenderResult(firstChild)) {
listContents = firstChild;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it is only taking the first child, users of the widget will need to wrap the content in a <virtual> element. Should this pass all the children so it could be used like the following?

<ListItem>
  Text node
  <span>sibling node</span>
</ListItem>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good question

} else {
const { leading = undefined, primary, trailing = undefined } = firstChild;
listContents = (
<virtual>
{leading ? <span classes={themedCss.leading}>{leading}</span> : undefined}
<span classes={themedCss.primary}>{primary}</span>
{trailing ? <span classes={themedCss.trailing}>{trailing}</span> : undefined}
{draggable && !trailing && (
<Icon
type="barsIcon"
classes={{ '@dojo/widgets/icon': { icon: [themedCss.dragIcon] } }}
theme={themeProp}
variant={variant}
/>
)}
</virtual>
);
}

let paddingClass: string | undefined;
switch (padding) {
case 'small':
paddingClass = themedCss.smallPadding;
break;
case 'medium':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the options be 'small' and 'large'?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could always add a large too, but that's fine by me

paddingClass = themedCss.mediumPadding;
break;
case 'none':
paddingClass = themedCss.noPadding;
break;
default:
break;
}

return (
<div
Expand All @@ -168,15 +205,15 @@ export const ListItem = listItemFactory(function ListItem({
classes={[
theme.variant(),
themedCss.root,
themedCss.height,
selected && themedCss.selected,
active && themedCss.active,
disabled && themedCss.disabled,
movedUp && themedCss.movedUp,
movedDown && themedCss.movedDown,
collapsed && themedCss.collapsed,
dragged && themedCss.dragged,
draggable && themedCss.draggable
draggable && themedCss.draggable,
paddingClass
]}
onclick={() => {
requestActive();
Expand All @@ -193,17 +230,7 @@ export const ListItem = listItemFactory(function ListItem({
ondrop={onDrop}
styles={{ visibility: dragged ? 'hidden' : undefined }}
>
{leading ? <span classes={themedCss.leading}>{leading}</span> : undefined}
<span classes={themedCss.primary}>{primary}</span>
{trailing ? <span classes={themedCss.trailing}>{trailing}</span> : undefined}
{draggable && !trailing && (
<Icon
type="barsIcon"
classes={{ '@dojo/widgets/icon': { icon: [themedCss.dragIcon] } }}
theme={themeProp}
variant={variant}
/>
)}
{listContents}
</div>
);
});
Expand Down
Loading