Skip to content

Commit

Permalink
fix(Form): fix reading and tab order when only FormItems are used (#5714
Browse files Browse the repository at this point in the history 
)

When a `Form` contains only `FormItems` and no `FormGroups`, the reading
and tab order of the form items should be first top to bottom, then
start to end.

Part of #5704
  • Loading branch information
@MarcusNotheis
MarcusNotheis committed Apr 16, 2024
1 parent f368e34 commit 5cec60b
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 65 deletions.
2 changes: 1 addition & 1 deletion packages/main/src/components/Form/Form.cy.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ describe('Form', () => {
cy.findByText('Toggle Input').click();
cy.findAllByText('Item 2').should('exist');
cy.findByTestId('2').should('be.visible').as('item2');
cy.get('@item2').parent().should('have.css', 'grid-column-start', '17').and('have.css', 'grid-row-start', '1');
cy.get('@item2').parent().should('have.css', 'grid-column-start', '5').and('have.css', 'grid-row-start', '2');

cy.findByText('Toggle Group').click();
cy.findByText('Group 1')
Expand Down
90 changes: 68 additions & 22 deletions packages/main/src/components/Form/Form.stories.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ export const Default: Story = {
render: (props) => {
return (
<Form {...props}>
<FormItem label="Sole Form Item">
<Input type={InputType.Text} />
</FormItem>
<FormGroup titleText="Personal Data">
<FormItem label="Name">
<Input type={InputType.Text} />
Expand Down Expand Up @@ -103,25 +100,6 @@ export const Default: Story = {
<CheckBox />
</FormItem>
</FormGroup>
<FormGroup titleText="Contact">
<FormItem label="Website">
<Link href={'https://sap.github.io/ui5-webcomponents-react'}>
https://sap.github.io/ui5-webcomponents-react
</Link>
</FormItem>
<FormItem label="Email">
<Link>some.one@sap.com</Link>
</FormItem>
<FormItem label="Slack">
<Link href={'https://openui5.slack.com/archives/CSQEJ2J04'}>#webcomponents-react</Link>
</FormItem>
<FormItem label="Company">
<Text>SAP</Text>
</FormItem>
<FormItem label="Company Headquarter">
<Text>Walldorf, Germany</Text>
</FormItem>
</FormGroup>
</Form>
);
}
Expand Down Expand Up @@ -330,3 +308,71 @@ export const DisplayEditMode: Story = {
);
}
};

export const FormWithOneGroup: Story = {
args: {
titleText: 'Address',
columnsM: 2,
columnsL: 3,
columnsXL: 4,
labelSpanS: 12,
labelSpanM: 12,
labelSpanL: 12,
labelSpanXL: 12,
children: null
},
render(props) {
return (
<Form {...props}>
<FormItem label="Name">
<Input id="name" />
</FormItem>

<FormItem label="Street/No">
<Input></Input>
</FormItem>

<FormItem label="ZIP Code/City">
<Input />
</FormItem>

<FormItem label="Country">
<Select id="country">
<Option value="England">England</Option>
<Option value="Germany">Germany</Option>
<Option value="USA">USA</Option>
</Select>
</FormItem>

<FormItem label="Web">
<Input type="URL" />
</FormItem>

<FormItem label="Twitter">
<Input />
</FormItem>

<FormItem label="Email">
<Input type="Email" />
</FormItem>

<FormItem label="Tel.">
<Input type="Tel" />
</FormItem>

<FormItem label="SMS">
<Input type="Tel" />
</FormItem>
<FormItem label="Mobile">
<Input type="Tel" />
</FormItem>
<FormItem label="Pager">
<Input type="Tel" />
</FormItem>
<FormItem label="Fax">
<Input type="Tel" />
</FormItem>
</Form>
);
}
};
117 changes: 75 additions & 42 deletions packages/main/src/components/Form/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ const recalcReducerFn = (prev: number) => {
return prev + 1;
};

type DisplayRange = 'Phone' | 'Tablet' | 'Desktop' | 'LargeDesktop';

export interface FormPropTypes extends CommonProps {
/**
* Components that are placed into Form.
Expand Down Expand Up @@ -129,13 +131,13 @@ const Form = forwardRef<HTMLFormElement, FormPropTypes>((props, ref) => {
const [items, setItems] = useState<Map<string, ItemInfo>>(() => new Map());
useStylesheet(styleData, Form.displayName);

const columnsMap = new Map();
const columnsMap = new Map<DisplayRange, number>();
columnsMap.set('Phone', columnsS);
columnsMap.set('Tablet', columnsM);
columnsMap.set('Desktop', columnsL);
columnsMap.set('LargeDesktop', columnsXL);

const labelSpanMap = new Map();
const labelSpanMap = new Map<DisplayRange, number>();
labelSpanMap.set('Phone', labelSpanS);
labelSpanMap.set('Tablet', labelSpanM);
labelSpanMap.set('Desktop', labelSpanL);
Expand Down Expand Up @@ -177,13 +179,14 @@ const Form = forwardRef<HTMLFormElement, FormPropTypes>((props, ref) => {
groupItem.formItemIds = new Set(groupItem.formItemIds).add(id);
} else {
clonedMap.set(groupId, {
id: groupId,
type: 'formGroup',
formItemIds: new Set([id])
});
}
} else {
if (!clonedMap.has(id)) {
clonedMap.set(id, { type, formItemIds: new Set() });
clonedMap.set(id, { id, type, formItemIds: new Set() });
}
}
return clonedMap;
Expand All @@ -210,55 +213,85 @@ const Form = forwardRef<HTMLFormElement, FormPropTypes>((props, ref) => {
const formGroups: FormGroupLayoutInfo[] = [];

let index = -1;
let localColumnIndex = 0;
// depending on the labelSpan, each form item takes up either 1 (labelSpan < 12) or 2 (labelSpan == 12) rows
const rowsPerFormItem = currentLabelSpan === 12 ? 2 : 1;
let rowIndex = (titleText ? 2 : 1) + rowsPerFormItem - 1;
// no. of rows in a "line" - e.g. when a group has 5 items, the next line needs to start below that group
let nextRowIndex = rowIndex;
let nextRowIndex = (titleText ? 2 : 1) + rowsPerFormItem - 1;
const rowsWithGroup = {};

items.forEach(({ type, formItemIds }, id) => {
const columnIndex = localColumnIndex % currentNumberOfColumns;
index++;
if (type === 'formGroup') {
rowsWithGroup[rowIndex] = true;
formGroups.push({ id, index, columnIndex, rowIndex });
let localRowIndex = 1;
let localIndex = 1;
const columnsWithItems: ItemInfo[][] = [];
const rowsPerColumn = Math.ceil(items.size / currentNumberOfColumns);

if (!formItemIds.size) {
nextRowIndex++;
}
formItemIds.forEach((itemId, _, set) => {
formItems.push({
id: itemId,
index,
groupId: id,
columnIndex,
rowIndex: rowIndex + localRowIndex,
lastGroupItem: set.size === localIndex
});
if (set.size === localIndex) {
if (nextRowIndex < rowIndex + localRowIndex + rowsPerFormItem) {
nextRowIndex = rowIndex + localRowIndex + rowsPerFormItem;
const allItemsArray = Array.from(items.entries());
const onlyFormItems = allItemsArray.every(([, item]) => item.type === 'formItem');

allItemsArray.forEach(([id, item], idx) => {
// when only FormItems are used, the Form should build up from top to bottom, then left to right
// when FormGroups are used, the Form should build up from left to right, then top to bottom
const localColumnIndex = onlyFormItems ? Math.floor(idx / rowsPerColumn) : idx % currentNumberOfColumns;
columnsWithItems[localColumnIndex] ??= [];
columnsWithItems[localColumnIndex].push({ id, ...item });
});

if (onlyFormItems) {
// if the last column only contains one row, balance it with the previous column
if (columnsWithItems.at(-1)?.length === 1 && columnsWithItems.at(-2)?.length > 1) {
columnsWithItems.at(-1).unshift(columnsWithItems.at(-2).pop());
}
columnsWithItems.forEach((columnItems, columnIndex) => {
let rowIndex = nextRowIndex;
columnItems.forEach(({ id }) => {
index++;
formItems.push({ id, index, columnIndex, rowIndex });
rowIndex += rowsPerFormItem;
});
});
} else {
let localColumnIndex = 0;
let rowIndex = (titleText ? 2 : 1) + rowsPerFormItem - 1;

items.forEach(({ type, formItemIds }, id) => {
const columnIndex = localColumnIndex % currentNumberOfColumns;
index++;
if (type === 'formGroup') {
rowsWithGroup[rowIndex] = true;
formGroups.push({ id, index, columnIndex, rowIndex });
let localRowIndex = 1;
let localIndex = 1;

if (!formItemIds.size) {
nextRowIndex++;
}
formItemIds.forEach((itemId, _, set) => {
formItems.push({
id: itemId,
index,
groupId: id,
columnIndex,
rowIndex: rowIndex + localRowIndex,
lastGroupItem: set.size === localIndex
});
if (set.size === localIndex) {
if (nextRowIndex < rowIndex + localRowIndex + rowsPerFormItem) {
nextRowIndex = rowIndex + localRowIndex + rowsPerFormItem;
}
}
localRowIndex += rowsPerFormItem;
localIndex++;
});
} else {
if (nextRowIndex < rowIndex + 1) {
nextRowIndex += rowsPerFormItem;
}
localRowIndex += rowsPerFormItem;
localIndex++;
});
} else {
if (nextRowIndex < rowIndex + 1) {
nextRowIndex += rowsPerFormItem;
formItems.push({ id, index, columnIndex, rowIndex });
}
formItems.push({ id, index, columnIndex, rowIndex });
}

if ((localColumnIndex + 1) % currentNumberOfColumns === 0) {
rowIndex = nextRowIndex;
}
localColumnIndex++;
});
if ((localColumnIndex + 1) % currentNumberOfColumns === 0) {
rowIndex = nextRowIndex;
}
localColumnIndex++;
});
}

return { formItems, formGroups, registerItem, unregisterItem, rowsWithGroup };
}, [items, registerItem, unregisterItem, currentNumberOfColumns, titleText, currentLabelSpan]);
Expand Down
1 change: 1 addition & 0 deletions packages/main/src/components/Form/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export type GroupContextType = {
};

export type ItemInfo = {
id: string;
type: FormElementTypes;
formItemIds: Set<string>;
};

0 comments on commit 5cec60b

Please sign in to comment.