fix: available columns calculation in assigning width to fill widgets (#22688)
## Description 1. On smaller viewports, space can not be equally divided between fill widgets because it is less than the min width of some. Width allocation is done in descending order of min width requirement of fill widgets. Allocation width is now being deducted from available columns to fix the logic. 2. Disable widget grouping on auto layout canvas. Fixes # 1. https://github.com/appsmithorg/appsmith/issues/22555 3. https://github.com/appsmithorg/appsmith/issues/22234 ## Type of change - Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? - Manual - Jest - ## Checklist: ### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag
This commit is contained in:
Preet Sidhu
GitHub
parent
167c1b3c3c
commit
b4ec1d9d5b
|
|
@ -2002,7 +2002,9 @@ function* addSuggestedWidget(action: ReduxAction<Partial<WidgetProps>>) {
|
|||
export function* groupWidgetsSaga() {
|
||||
const selectedWidgetIDs: string[] = yield select(getSelectedWidgets);
|
||||
const isMultipleWidgetsSelected = selectedWidgetIDs.length > 1;
|
||||
|
||||
// Grouping functionality has been temporarily disabled for auto layout canvas.
|
||||
const isAutoLayout: boolean = yield select(getIsAutoLayout);
|
||||
if (isAutoLayout) return;
|
||||
if (isMultipleWidgetsSelected) {
|
||||
try {
|
||||
yield put({
|
||||
|
|
|
|||
|
|
@ -17,6 +17,8 @@ import {
|
|||
updateWidgetPositions,
|
||||
} from "./positionUtils";
|
||||
import { AppPositioningTypes } from "reducers/entityReducers/pageListReducer";
|
||||
import { LabelPosition } from "components/constants";
|
||||
import * as utils from "./flexWidgetUtils";
|
||||
|
||||
describe("test PositionUtils methods", () => {
|
||||
const mainCanvasWidth = 960;
|
||||
|
|
@ -166,6 +168,159 @@ describe("test PositionUtils methods", () => {
|
|||
.fillWidgetLength,
|
||||
).toEqual(24);
|
||||
});
|
||||
it("should allocate columns for fill widgets that match min widths if enough space is unavailable", () => {
|
||||
const widgets = {
|
||||
"1": {
|
||||
widgetId: "1",
|
||||
leftColumn: 0,
|
||||
rightColumn: 32,
|
||||
alignment: FlexLayerAlignment.Start,
|
||||
topRow: 0,
|
||||
bottomRow: 4,
|
||||
type: "DOCUMENT_VIEWER_WIDGET",
|
||||
widgetName: "",
|
||||
renderMode: RenderModes.CANVAS,
|
||||
version: 1,
|
||||
parentColumnSpace: 10,
|
||||
parentRowSpace: 10,
|
||||
isLoading: false,
|
||||
responsiveBehavior: ResponsiveBehavior.Hug,
|
||||
autoLayout: {
|
||||
widgetSize: [
|
||||
{
|
||||
viewportMinWidth: 0,
|
||||
configuration: () => {
|
||||
return {
|
||||
minWidth: "280px",
|
||||
minHeight: "280px",
|
||||
};
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
"2": {
|
||||
widgetId: "2",
|
||||
leftColumn: 32,
|
||||
rightColumn: 64,
|
||||
alignment: FlexLayerAlignment.Start,
|
||||
topRow: 0,
|
||||
bottomRow: 7,
|
||||
type: "CURRENCY_INPUT_WIDGET",
|
||||
widgetName: "Currency1",
|
||||
renderMode: RenderModes.CANVAS,
|
||||
version: 1,
|
||||
parentColumnSpace: 10,
|
||||
parentRowSpace: 10,
|
||||
isLoading: false,
|
||||
responsiveBehavior: ResponsiveBehavior.Fill,
|
||||
autoLayout: {
|
||||
disabledPropsDefaults: {
|
||||
labelPosition: LabelPosition.Top,
|
||||
labelTextSize: "0.875rem",
|
||||
},
|
||||
defaults: {
|
||||
rows: 6.6,
|
||||
},
|
||||
autoDimension: {
|
||||
height: true,
|
||||
},
|
||||
widgetSize: [
|
||||
{
|
||||
viewportMinWidth: 0,
|
||||
configuration: () => {
|
||||
return {
|
||||
minWidth: "120px",
|
||||
};
|
||||
},
|
||||
},
|
||||
],
|
||||
disableResizeHandles: {
|
||||
vertical: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
"3": {
|
||||
widgetId: "3",
|
||||
leftColumn: 64,
|
||||
rightColumn: 96,
|
||||
alignment: FlexLayerAlignment.End,
|
||||
topRow: 0,
|
||||
bottomRow: 7,
|
||||
type: "CONTAINER_WIDGET",
|
||||
widgetName: "Container1",
|
||||
renderMode: RenderModes.CANVAS,
|
||||
version: 1,
|
||||
parentColumnSpace: 10,
|
||||
parentRowSpace: 10,
|
||||
isLoading: false,
|
||||
responsiveBehavior: ResponsiveBehavior.Fill,
|
||||
},
|
||||
};
|
||||
jest
|
||||
.spyOn(utils, "getWidgetMinMaxDimensionsInPixel")
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
.mockImplementation((widget: any, width: number) => {
|
||||
if (
|
||||
["DOCUMENT_VIEWER_WIDGET", "CONTAINER_WIDGET"].includes(
|
||||
widget?.type,
|
||||
)
|
||||
)
|
||||
return {
|
||||
minWidth: 280,
|
||||
minHeight: 280,
|
||||
maxWidth: undefined,
|
||||
maxHeight: undefined,
|
||||
};
|
||||
if (widget?.type === "CURRENCY_INPUT_WIDGET")
|
||||
return {
|
||||
minWidth: 120,
|
||||
minHeight: 40,
|
||||
maxWidth: undefined,
|
||||
maxHeight: undefined,
|
||||
};
|
||||
return {
|
||||
minWidth: undefined,
|
||||
minHeight: undefined,
|
||||
maxWidth: undefined,
|
||||
maxHeight: undefined,
|
||||
};
|
||||
});
|
||||
const layer: FlexLayer = {
|
||||
children: [
|
||||
{ id: "1", align: FlexLayerAlignment.Start },
|
||||
{ id: "2", align: FlexLayerAlignment.Start },
|
||||
{ id: "3", align: FlexLayerAlignment.End },
|
||||
],
|
||||
};
|
||||
const result = extractAlignmentInfo(widgets, layer, false, 600, 1, false);
|
||||
|
||||
/**
|
||||
* Canvas width = 600
|
||||
* # of fill widgets = 3
|
||||
* standard fill widget length (f) = 600 / 3 = 200
|
||||
* min widths in descending order -> 280, 280, 120.
|
||||
*
|
||||
* In descending order of min widths:
|
||||
* available space: 600
|
||||
* 1st fill widget length (DocumentViewer) -> 280 > 200 -> 280
|
||||
* available space: 600 - 280 = 320
|
||||
* standard fill widget length (f) = 320 / 2 = 160
|
||||
*
|
||||
* 2nd fill widget length (ContainerWidget) -> 280 > 160 -> 280
|
||||
* available space: 320 - 280 = 40
|
||||
* standard fill widget length (f) = 40 / 1 = 40
|
||||
*
|
||||
* 3rd fill widget length (CurrencyInput) -> 120 > 40 -> 120
|
||||
*
|
||||
* => widgets will overflow the canvas.
|
||||
*/
|
||||
|
||||
// DocumnetViewer + CurrencyInput
|
||||
expect(result.info[0].columns).toEqual(280 + 120);
|
||||
// ContainerWidget
|
||||
expect(result.info[2].columns).toEqual(280);
|
||||
});
|
||||
});
|
||||
|
||||
describe("test getAlignmentSizeinfo method", () => {
|
||||
|
|
@ -589,6 +744,317 @@ describe("test PositionUtils methods", () => {
|
|||
expect(result.widgets["3"].mobileLeftColumn).toEqual(48);
|
||||
expect(result.widgets["3"].mobileRightColumn).toEqual(64);
|
||||
});
|
||||
|
||||
it("should allocate columns for fill widgets in descending order of their min width requirement", () => {
|
||||
const widgets = {
|
||||
"1": {
|
||||
widgetId: "1",
|
||||
leftColumn: 0,
|
||||
rightColumn: 32,
|
||||
alignment: FlexLayerAlignment.Start,
|
||||
topRow: 0,
|
||||
bottomRow: 4,
|
||||
type: "DOCUMENT_VIEWER_WIDGET",
|
||||
widgetName: "",
|
||||
renderMode: RenderModes.CANVAS,
|
||||
version: 1,
|
||||
parentColumnSpace: 10,
|
||||
parentRowSpace: 10,
|
||||
isLoading: false,
|
||||
responsiveBehavior: ResponsiveBehavior.Fill,
|
||||
autoLayout: {
|
||||
widgetSize: [
|
||||
{
|
||||
viewportMinWidth: 0,
|
||||
configuration: () => {
|
||||
return {
|
||||
minWidth: "280px",
|
||||
minHeight: "280px",
|
||||
};
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
"2": {
|
||||
widgetId: "2",
|
||||
leftColumn: 32,
|
||||
rightColumn: 64,
|
||||
alignment: FlexLayerAlignment.Start,
|
||||
topRow: 0,
|
||||
bottomRow: 7,
|
||||
type: "CURRENCY_INPUT_WIDGET",
|
||||
widgetName: "Currency1",
|
||||
renderMode: RenderModes.CANVAS,
|
||||
version: 1,
|
||||
parentColumnSpace: 10,
|
||||
parentRowSpace: 10,
|
||||
isLoading: false,
|
||||
responsiveBehavior: ResponsiveBehavior.Fill,
|
||||
autoLayout: {
|
||||
disabledPropsDefaults: {
|
||||
labelPosition: LabelPosition.Top,
|
||||
labelTextSize: "0.875rem",
|
||||
},
|
||||
defaults: {
|
||||
rows: 6.6,
|
||||
},
|
||||
autoDimension: {
|
||||
height: true,
|
||||
},
|
||||
widgetSize: [
|
||||
{
|
||||
viewportMinWidth: 0,
|
||||
configuration: () => {
|
||||
return {
|
||||
minWidth: "120px",
|
||||
};
|
||||
},
|
||||
},
|
||||
],
|
||||
disableResizeHandles: {
|
||||
vertical: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
"3": {
|
||||
widgetId: "3",
|
||||
leftColumn: 64,
|
||||
rightColumn: 96,
|
||||
alignment: FlexLayerAlignment.End,
|
||||
topRow: 0,
|
||||
bottomRow: 7,
|
||||
type: "CONTAINER_WIDGET",
|
||||
widgetName: "Container1",
|
||||
renderMode: RenderModes.CANVAS,
|
||||
version: 1,
|
||||
parentColumnSpace: 10,
|
||||
parentRowSpace: 10,
|
||||
isLoading: false,
|
||||
responsiveBehavior: ResponsiveBehavior.Fill,
|
||||
},
|
||||
};
|
||||
jest
|
||||
.spyOn(utils, "getWidgetMinMaxDimensionsInPixel")
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
.mockImplementation((widget: any, width: number) => {
|
||||
if (
|
||||
["DOCUMENT_VIEWER_WIDGET", "CONTAINER_WIDGET"].includes(
|
||||
widget?.type,
|
||||
)
|
||||
)
|
||||
return {
|
||||
minWidth: 280,
|
||||
minHeight: 280,
|
||||
maxWidth: undefined,
|
||||
maxHeight: undefined,
|
||||
};
|
||||
if (widget?.type === "CURRENCY_INPUT_WIDGET")
|
||||
return {
|
||||
minWidth: 120,
|
||||
minHeight: 40,
|
||||
maxWidth: undefined,
|
||||
maxHeight: undefined,
|
||||
};
|
||||
return {
|
||||
minWidth: undefined,
|
||||
minHeight: undefined,
|
||||
maxWidth: undefined,
|
||||
maxHeight: undefined,
|
||||
};
|
||||
});
|
||||
const layer: FlexLayer = {
|
||||
children: [
|
||||
{ id: "1", align: FlexLayerAlignment.Start },
|
||||
{ id: "2", align: FlexLayerAlignment.Start },
|
||||
{ id: "3", align: FlexLayerAlignment.End },
|
||||
],
|
||||
};
|
||||
const alignmentInfo = extractAlignmentInfo(
|
||||
widgets,
|
||||
layer,
|
||||
false,
|
||||
640,
|
||||
10,
|
||||
false,
|
||||
);
|
||||
const res = placeWidgetsWithoutWrap(
|
||||
widgets,
|
||||
alignmentInfo.info,
|
||||
0,
|
||||
false,
|
||||
);
|
||||
|
||||
/**
|
||||
* available columns: 64
|
||||
* column space: 10
|
||||
* # of fill widgets = 3
|
||||
* standard fill widget length (f) = 64 / 3 = 21.3333
|
||||
* min widths in descending order -> 28 (minWidth / columnSpace), 28, 12.
|
||||
*
|
||||
* In descending order of min widths:
|
||||
* available columns: 64
|
||||
* 1st fill widget length (DocumentViewer) -> 28 > 21.3333 -> 28
|
||||
* available columns: 64 - 28 = 36
|
||||
* standard fill widget length (f) = 36 / 2 = 18
|
||||
*
|
||||
* 2nd fill widget length (ContainerWidget) -> 28 > 18 -> 28
|
||||
* available columns: 36 - 28 = 8
|
||||
* standard fill widget length (f) = 8 / 1 = 8
|
||||
*
|
||||
* 3rd fill widget length (CurrencyInput) -> 12 > 8 -> 12
|
||||
*
|
||||
* => widgets will overflow the canvas.
|
||||
* => min widths of each widget is respected.
|
||||
*/
|
||||
|
||||
// DocumentViewer
|
||||
expect(res.widgets["1"].leftColumn).toEqual(0);
|
||||
expect(res.widgets["1"].rightColumn).toEqual(28);
|
||||
// CurrencyInput
|
||||
expect(res.widgets["2"].leftColumn).toEqual(28);
|
||||
expect(res.widgets["2"].rightColumn).toEqual(40);
|
||||
// ContainerWidget
|
||||
expect(res.widgets["3"].leftColumn).toEqual(40);
|
||||
expect(res.widgets["3"].rightColumn).toEqual(68);
|
||||
});
|
||||
|
||||
it("should allocate columns for fill widgets in descending order of their min width requirement - Part 2", () => {
|
||||
const widgets = {
|
||||
"1": {
|
||||
widgetId: "1",
|
||||
leftColumn: 0,
|
||||
rightColumn: 21.3333,
|
||||
alignment: FlexLayerAlignment.Start,
|
||||
topRow: 0,
|
||||
bottomRow: 4,
|
||||
type: "DOCUMENT_VIEWER_WIDGET",
|
||||
widgetName: "",
|
||||
renderMode: RenderModes.CANVAS,
|
||||
version: 1,
|
||||
parentColumnSpace: 10,
|
||||
parentRowSpace: 10,
|
||||
isLoading: false,
|
||||
responsiveBehavior: ResponsiveBehavior.Fill,
|
||||
},
|
||||
"2": {
|
||||
widgetId: "2",
|
||||
leftColumn: 21.33333,
|
||||
rightColumn: 42.66666,
|
||||
alignment: FlexLayerAlignment.Start,
|
||||
topRow: 0,
|
||||
bottomRow: 7,
|
||||
type: "CURRENCY_INPUT_WIDGET",
|
||||
widgetName: "Currency1",
|
||||
renderMode: RenderModes.CANVAS,
|
||||
version: 1,
|
||||
parentColumnSpace: 10,
|
||||
parentRowSpace: 10,
|
||||
isLoading: false,
|
||||
responsiveBehavior: ResponsiveBehavior.Fill,
|
||||
},
|
||||
"3": {
|
||||
widgetId: "3",
|
||||
leftColumn: 42.66666,
|
||||
rightColumn: 63.99999,
|
||||
alignment: FlexLayerAlignment.Start,
|
||||
topRow: 0,
|
||||
bottomRow: 7,
|
||||
type: "CURRENCY_INPUT_WIDGET",
|
||||
widgetName: "Currency2",
|
||||
renderMode: RenderModes.CANVAS,
|
||||
version: 1,
|
||||
parentColumnSpace: 10,
|
||||
parentRowSpace: 10,
|
||||
isLoading: false,
|
||||
responsiveBehavior: ResponsiveBehavior.Fill,
|
||||
},
|
||||
};
|
||||
jest
|
||||
.spyOn(utils, "getWidgetMinMaxDimensionsInPixel")
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
.mockImplementation((widget: any, width: number) => {
|
||||
if (
|
||||
["DOCUMENT_VIEWER_WIDGET", "CONTAINER_WIDGET"].includes(
|
||||
widget?.type,
|
||||
)
|
||||
)
|
||||
return {
|
||||
minWidth: 280,
|
||||
minHeight: 280,
|
||||
maxWidth: undefined,
|
||||
maxHeight: undefined,
|
||||
};
|
||||
if (widget?.type === "CURRENCY_INPUT_WIDGET")
|
||||
return {
|
||||
minWidth: 120,
|
||||
minHeight: 40,
|
||||
maxWidth: undefined,
|
||||
maxHeight: undefined,
|
||||
};
|
||||
return {
|
||||
minWidth: undefined,
|
||||
minHeight: undefined,
|
||||
maxWidth: undefined,
|
||||
maxHeight: undefined,
|
||||
};
|
||||
});
|
||||
const layer: FlexLayer = {
|
||||
children: [
|
||||
{ id: "1", align: FlexLayerAlignment.Start },
|
||||
{ id: "2", align: FlexLayerAlignment.Start },
|
||||
{ id: "3", align: FlexLayerAlignment.End },
|
||||
],
|
||||
};
|
||||
const alignmentInfo = extractAlignmentInfo(
|
||||
widgets,
|
||||
layer,
|
||||
false,
|
||||
640,
|
||||
10,
|
||||
false,
|
||||
);
|
||||
const res = placeWidgetsWithoutWrap(
|
||||
widgets,
|
||||
alignmentInfo.info,
|
||||
0,
|
||||
false,
|
||||
);
|
||||
|
||||
/**
|
||||
* total available columns = 64
|
||||
* # of fill widgets = 3
|
||||
* columnSpace = 10
|
||||
* standard fill widget length (f) = 64 / 3 = 21.333
|
||||
* min widths in descending order of columns -> 28 (280 / 10), 12, 12.
|
||||
*
|
||||
* In descending order of min widths:
|
||||
* available columns: 64
|
||||
* 1st fill widget length (DocumentViewer) -> 28 > 21.3333 -> 28
|
||||
* available columns: 64 - 28 = 36
|
||||
* standard fill widget length (f) = 36 / 2 = 18
|
||||
*
|
||||
* 2nd fill widget length (CurrencyInput) -> 12 < 18 -> 18
|
||||
* available columns: 36 - 18 = 18
|
||||
* standard fill widget length (f) = 18 / 1 = 18
|
||||
*
|
||||
* 3rd fill widget length (CurrencyInput) -> 12 < 18 -> 180
|
||||
*
|
||||
* => widgets don't overflow the canvas.
|
||||
* => DocumentViewer gets more columns (28) to address its min width requirement.
|
||||
* => rest of the space gets evenly distributed among the remaining fill widgets, as the it is larger than their min width requirement.
|
||||
*/
|
||||
|
||||
// DocumentViewer
|
||||
expect(res.widgets["1"].leftColumn).toEqual(0);
|
||||
expect(res.widgets["1"].rightColumn).toEqual(28);
|
||||
// CurrencyInput1
|
||||
expect(res.widgets["2"].leftColumn).toEqual(28);
|
||||
expect(res.widgets["2"].rightColumn).toEqual(46);
|
||||
// CurrencyInput2
|
||||
expect(res.widgets["3"].leftColumn).toEqual(46);
|
||||
expect(res.widgets["3"].rightColumn).toEqual(64);
|
||||
});
|
||||
});
|
||||
|
||||
describe("test placeWrappedWidgets method", () => {
|
||||
|
|
|
|||
|
|
@ -401,7 +401,7 @@ export function extractAlignmentInfo(
|
|||
}
|
||||
}
|
||||
|
||||
const availableColumns: number =
|
||||
let availableColumns: number =
|
||||
GridDefaults.DEFAULT_GRID_COLUMNS -
|
||||
startColumns -
|
||||
centerColumns -
|
||||
|
|
@ -416,13 +416,11 @@ export function extractAlignmentInfo(
|
|||
.sort((a, b) => b.columns - a.columns)
|
||||
.forEach((each, index) => {
|
||||
const { child, columns } = each;
|
||||
let width = fillWidgetLength;
|
||||
// If the fill widget's minWidth is greater than the available space, then assign the widget's minWidth as its width.
|
||||
if (columns > fillWidgetLength) {
|
||||
width = columns;
|
||||
fillWidgetLength =
|
||||
(availableColumns - width) / (fillChildren.length - index - 1);
|
||||
}
|
||||
// If minWidth > availableColumns / # of fill widgets => assign columns based on minWidth and update fillLength for remaining fill widgets.
|
||||
const width = columns > fillWidgetLength ? columns : fillWidgetLength;
|
||||
availableColumns = Math.max(availableColumns - width, 0);
|
||||
if (fillChildren.length - index - 1 > 0)
|
||||
fillWidgetLength = availableColumns / (fillChildren.length - index - 1);
|
||||
if (child.align === FlexLayerAlignment.Start) {
|
||||
startColumns += width;
|
||||
const index = startChildren.findIndex(
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user