Dragging new item to bottom of scrollable container causes jank

[humphreybc]

Bug or feature request?

Either a bug or something currently unsupported in our implementation. Probably related to #131 and #338 although I'm not sure if it's a duplicate of either of these.

Expected behavior

Dragging a Draggable to the bottom of a scrollable Droppable container should grow the scrollable Droppable to accommodate the placeholder of the new item, even when there's something at the bottom like a create button. Here's an example of it working well in Trello:

Actual behavior

Dragging a Draggable to the bottom of a scrollable Droppable container causes the Droppable container to jank, as it tries to grow Droppable but the parent container does not grow, I think:

Steps to reproduce

Here's the relevant code. I'm not sure if this is a bug or a symptom of #131. Perhaps someone could point me in the right direction and we might be able to change our markup to work around the issue. Appreciate the help!

Browser version

Chrome 66.0.3359.181 on Mac OS X 10.13.4.

[DimitarNestorov]

Try this:

import * as csx from "csx";
import { types } from "dovetail/graphql";
import { NoteGroupColor } from "dovetail/types";
import { NoteSortDropdownChoice } from "dovetail/ui/dropdown/NoteSortDropdown";
import { OutsideClick } from "dovetail/ui/layout/OutsideClick";
import { NewNoteItem } from "dovetail/ui/note";
import { CreateNoteItem } from "dovetail/ui/note/CreateNoteItem";
import { Cache } from "dovetail/ui/util/Cache";
import { makeStyleSafe } from "dovetail/util/reactBeautifulDnd";
import * as text from "dovetail/util/text";
import { BORDER_RADIUS, BOX_SHADOW_FOCUS, COLORS } from "dovetail/variables";
import React from "react";
import { Draggable, Droppable } from "react-beautiful-dnd";
import { styled } from "typestyle-react";
import { NoteGroupHeader } from "./NoteGroupHeader";

const enum DragType {
  NOTE = "NOTE"
}

interface Props {
  collapsed: boolean;
  color: NoteGroupColor;
  focusTitle?: boolean;
  isLifted: boolean;
  items: Array<{
    id: string;
    render: () => React.ReactElement<{}>;
  }>;
  layout: "LIST" | "BOARD";
  noteGroupId: string;
  noteType: types.NoteType;
  onCollapsedChange: (newCollapsed: boolean) => void;
  onColorChange: (newColor: NoteGroupColor) => void;
  onDelete: () => void;
  onNoteCreate: (title: string, positionAtTop: boolean) => void;
  onSort: (type: NoteSortDropdownChoice) => void;
  onTitleSave: (newValue: string) => void;
  title: string;
}

interface State {
  creating: boolean;
  newNoteTitle: string;
  positionNewNoteAtTop: boolean;
}

export class NoteGroup extends React.PureComponent<Props, State> {
  public state: State = {
    creating: false,
    newNoteTitle: "",
    positionNewNoteAtTop: false
  };

  public render() {
    const {
      collapsed,
      color,
      focusTitle,
      isLifted,
      items,
      layout,
      noteGroupId,
      noteType,
      onColorChange,
      onDelete,
      onSort,
      onTitleSave,
      title
    } = this.props;

    const { creating, positionNewNoteAtTop } = this.state;

    const createNoteItem = (
      <NoteGroupItem>
        <OutsideClick onOutsideClick={this.toggleCreating}>
          <CreateNoteItem
            layout={layout}
            onChange={e => {
              this.setState({ newNoteTitle: e.target.value });
            }}
            onDismiss={this.toggleCreating}
            onPositionChange={() => {
              this.setState({ positionNewNoteAtTop: !this.state.positionNewNoteAtTop });
            }}
            onSave={this.createNote}
            placeholder={text.smartNoteTitle("", noteType)}
            positionAtTop={this.state.positionNewNoteAtTop}
            title={this.state.newNoteTitle}
          />
        </OutsideClick>
      </NoteGroupItem>
    );

    return (
      <GroupContainer styled={{ color }} tabIndex={0}>
        <NoteGroupHeader
          collapsed={collapsed}
          color={color}
          focusTitle={focusTitle}
          layout={layout}
          onCollapseExpandClick={this.handleCollapseExpandClick}
          onColorChange={onColorChange}
          onCreateClick={() => {
            this.setState({
              creating: true,
              positionNewNoteAtTop: true
            });
          }}
          onDelete={onDelete}
          onSort={onSort}
          onTitleSave={onTitleSave}
          title={title}
          totalCount={items.length}
        />
        {!collapsed ? (
          <>
            <Content>
              {creating && positionNewNoteAtTop === true ? createNoteItem : null}
              <Droppable droppableId={noteGroupId} type={DragType.NOTE}>
                {({ innerRef, placeholder }, { isDraggingOver }) => (
                  <div ref={innerRef}>
                    <Cache
                      shouldUpdate={isLifted === false}
                      render={() => {
                        if (collapsed) {
                          return null;
                        }

                        if (items.length > 0) {
                          const children = items.map((item, index) => (
                            <Draggable key={item.id} draggableId={item.id} index={index}>
                              {({ draggableProps, dragHandleProps, innerRef }) => {
                                // https://github.com/DefinitelyTyped/DefinitelyTyped/pull/22902/files#r162243798
                                const { style, ...nonStyleDraggableProps } = draggableProps;
                                const customDragHandleProps = { ...dragHandleProps, tabIndex: -1 };

                                return (
                                  <NoteGroupItem
                                    innerRef={innerRef}
                                    style={makeStyleSafe(style)}
                                    {...nonStyleDraggableProps}
                                    {...customDragHandleProps}
                                  >
                                    {item.render()}
                                  </NoteGroupItem>
                                );
                              }}
                            </Draggable>
                          ));
                          placeholder && children.push(React.cloneElement(placeholder, {key: 'placeholder'}));
                          return children;
                        } else if (creating === false) {
                          return <MinHeight styled={{ isDraggingOver }} />;
                        } else {
                          return null;
                        }
                      }}
                    />
                  </div>
                )}
              </Droppable>
              {creating && positionNewNoteAtTop === false ? createNoteItem : null}
            </Content>
            <NewNoteItem onClick={this.toggleCreating} color={color} type={noteType} />
          </>
        ) : null}
      </GroupContainer>
    );
  }

  private readonly handleCollapseExpandClick = () => {
    this.props.onCollapsedChange(!this.props.collapsed);
  };

  private readonly toggleCreating = () => {
    this.setState({ creating: !this.state.creating });
  };

  private readonly createNote = () => {
    this.props.onNoteCreate(this.state.newNoteTitle, this.state.positionNewNoteAtTop);
    this.setState({ newNoteTitle: "" });
  };
}

const NoteGroupItem = styled("div", {
  marginTop: "8px",
  outline: "none"
});

const GroupContainer = styled("div", ({ color }: { color: NoteGroupColor }) => ({
  backgroundColor:
    color !== null
      ? csx
          .color(color)
          .tint(0.9)
          .toString()
      : COLORS.i04,
  borderRadius: BORDER_RADIUS,
  color: color !== null ? COLORS.white : COLORS.i60,
  display: "flex",
  flexDirection: "column",
  maxHeight: "100%",
  outline: "none",
  overflow: "hidden",

  $nest: {
    "&:focus": {
      boxShadow: BOX_SHADOW_FOCUS
    }
  }
}));

const Content = styled("div", {
  overflowY: "auto",
  overflowX: "hidden",
  padding: "0 8px 8px"
});

const MinHeight = styled("div", ({ isDraggingOver }: { isDraggingOver: boolean }) => ({
  height: isDraggingOver ? "84px" : "48px"
}));

From the readme (this section):

• provided.placeholder: This is used to create space in the Droppable as needed during a drag. This space is needed when a user is dragging over a list that is not the home list. Please be sure to put the placeholder inside of the component for which you have provided the ref. We need to increase the size of the Droppable itself. This is different from Draggable where the placeholder needs to be a sibling to the draggable node.

[humphreybc]

Thanks @DimitarNestorov – we got confused by the 7.0.0 changelog which says:

We no longer require you to insert a sibling placeholder element for a Draggable

[humphreybc]

@DimitarNestorov cloneElement only works with elements, and placeholder is a ReactNode. Is there a reason why you're using cloneElement? Because this also seems to get the desired behavior:

children.push(placeholder);
return children;

While adding the placeholder fixes the janky scroll problem, there are still two more issues related to the placeholder and scrollable containers:

1. The placeholder does not expand the container when dragging an item into an empty container.
2. Dragging an item to the end of the list is difficult because there appears to be a tiny drop zone.

Both problems can be seen in this gif:

Are these duplicates of outstanding issues, or should I open new ones?

[DimitarNestorov]

Blind coding is the general reason.
Don't you get the warning: "every element in an array should have a key"? Probably not because it's a node.

[humphreybc]

Blind coding is the general reason.

Fair enough! 😀

Don't you get the warning: "every element in an array should have a key"? Probably not because it's a node.

Yeah, I think because they're nodes it's okay.

[alexreardon]

@humphreybc, the droppable is not expanding as I expect you have no put a minimum height on it. If it has a height of 0 then the user will never be able to drag over it

[alexreardon]

Is there anything outstanding here? Was the issue that the placeholder was not being added?

[humphreybc]

@alexreardon Yep, the issue was the placeholder not being added. Do you have any thoughts on the two issues I mentioned in this comment?

The placeholder does not expand the container when dragging an item into an empty container.
Dragging an item to the end of the list is difficult because there appears to be a tiny drop zone.

Not sure if I should open new issues.

[alexreardon]

Feel free to. I think both of those things should be working, but if they are not then I can take a further look

…

On Wed, 6 Jun 2018 at 9:57 pm, Benjamin Humphrey ***@***.***> wrote: @alexreardon <https://github.com/alexreardon> Yep, the issue was the placeholder not being added. Do you have any thoughts on the two issues I mentioned in this comment <#533 (comment)> ? The placeholder does not expand the container when dragging an item into an empty container. Dragging an item to the end of the list is difficult because there appears to be a tiny drop zone. Not sure if I should open new issues. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#533 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACFN7VG7faRRR5iJU8uL5K3PBTpzJoaTks5t58O6gaJpZM4UUYyY> .

[alexreardon]

I will close this issue. Please create other ones if things are not working how you would expect @humphreybc. I am happy to look into it further.