Features/cgu/24.2/isd #984

cguglielmo · 2024-04-25T10:38:36Z

No description provided.

Also, don't fail if there is no parent, because tiles may be created without one (e.g. by hybrid manager for a scout js tile grid)

Focus manager calls preventDefault() if user presses mouse down on a non-focusable element to ensure the currently active element does not lose focus. Unfortunately, this also prevents the dragstart event which is needed if a widget requires drag & drop support.

The method already works correctly. The new early === check may be a tiny bit faster. The other changes are just code style related.

FormFieldMenu uses the ColumnLayout which makes the widgets as width as they prefer to be. This means the size may change if an error status appears or the label changes so the parent needs to be invalidated. This does currently not happen because HtmlComponent.isValidatorRoot() returns true if layoutData.useUiWidth is set to false. This is the case here because a LogicalGridData is used instead of a ColumnLayoutData.

This enables automatic serialization / deserialization if the enum is used in a data object.

The returned jquery collection contained jquery objects instead of html elements which is different to jquery collection by jquery and prevents using methods like .on()

Makes it possible to add custom save needed behavior. Also removed saveNeeded from model because it is a computed property

If the resizable element is inside a scroll container, the scroll position needs to be considered when drawing the overlay

This ensures the resizable is always visible even on inverted tiles.

If there are many tests, the sandbox will be moved out of view which may break tests that need to interact with the sandbox elements, e.g. by clicking on them This fix reduces the height of the jasmine reporter section so the sandbox is always on top. Because of the overflowing, the content of the reporter section is still visible even though its height is 0.

Remove obsolete namespace in spec name. Remove obsolete jshint annotation. Remove DEFAULT_TIMEOUT_INTERVAL because it will affect every spec not only FocusManagerSpec and it does not seem to be needed.

This is useful if the splitter itself should be visualized in another way, e.g. by using one of the fields border.

Not every icon always has the same width. If icons are used in table cells along with text, the alignment of the texts in a column may be off. This typically affects list boxes or smart fields if the lookup rows have icons. The tree doesn't have that problem because it uses a min-width for the icons. The same logic is now applied to table cells as well.

HybridActionEvent uses actionType instead of eventType. Also move HybridEvent type to Adapter because it is only relevant for the communication with Java and not part of the hybrid manager api.

Sometimes the result object is already available. In that case it would be cumbersome to map the values into the given result object. With the new method the result object can be returned directly.

When importing the main index file from files in the testing folder, the import must not necessarily end with 'index'. For example if the index is in a folder called src/main/js, the import could be ../main/js. In fact, this how IntelliJ adds the import by default.

Codes delivered by the CodeResource are DOs and can therefor contain a _type property

If some of the tiles have h or w > 1 and others don't, the navigation between the tiles did not work properly.

cguglielmo · 2024-05-21T14:40:36Z

jenkins run tests

mvilliger · 2024-05-22T11:44:56Z

eclipse-scout-core/src/style/sizes.less

@@ -390,4 +390,5 @@
 /* -------------------------------- */
 @iphone5-height: 568px;
 @iphone5-width: 320px;
-@iphone6-width: 375px;
+@iphone6-width: 375px; // includes iphone 6, 6s, 6s plus, 7, 8


Looks good but the comment of this commit has a typo: sizes.lesS

mvilliger · 2024-05-22T12:09:11Z

org.eclipse.scout.rt.shared/src/main/java/org/eclipse/scout/rt/shared/ui/UserAgentUtility.java

@@ -56,4 +56,25 @@ public static UserAgent getCurrentUserAgent() {
    }
  }

+  /**


Is this method really necessary? It would also be possible to use the ClientRunContext instead?

eclipse-scout-core/src/layout/logicalgrid/LogicalGridLayout.ts

fschinkel · 2024-05-22T07:41:40Z

eclipse-scout-core/src/layout/logicalgrid/LogicalGridLayout.ts

+      this.spy?.dispose();
+      this.spy = null;
+    } else {
+      this.spy = new LogicalGridLayoutSpy();


trigger layout in order to draw the spies cell bounds?

this is not possible here since we don't have the $container. The idea is to use scout.setLogicalGridSpyEnabled

fschinkel · 2024-05-22T08:27:07Z

eclipse-scout-core/src/layout/logicalgrid/LogicalGridLayoutInfo.ts

@@ -228,77 +230,67 @@ export class LogicalGridLayoutInfo implements LogicalGridLayoutInfoModel {
          prefw = this.logicalWidthInPixel(cons);
        }
        prefw = Math.floor(prefw);
-        for (j = cons.gridx; j < cons.gridx + cons.gridw && j < this.cols; j++) {
+        for (let j = cons.gridx; j < cons.gridx + cons.gridw && j < this.cols; j++) {


Please simplify this loop. As cons.gridw is 1 this loop is pointless and can be replaced by a simple if-statement.

Suggested change

for (let j = cons.gridx; j < cons.gridx + cons.gridw && j < this.cols; j++) {

if (cons.gridx < this.cols) {

fschinkel · 2024-05-22T09:43:40Z

eclipse-scout-core/src/layout/logicalgrid/LogicalGridLayoutInfo.ts

        if (cons.widthHint > 0) {
-          distWidth = cons.widthHint - spanWidth - (hSpan - 1) * this.hgap;
+          distWidth[lc.PREF] = cons.widthHint - spanWidth[lc.PREF] - hGaps;


Please simplify:
distWidth[lc.PREF] will always be value - spanWidth[lc.PREF] - hGaps and distWidth[lc.MAX] is cons.maxWidth - spanWidth[lc.MAX] - hGaps.
Then distWidth[lc.PREF] and spanWidth[lc.PREF] (or distWidth[lc.MAX] and spanWidth[lc.MAX]) are passed to _distributeWidth where the values are summed up again.
So it will be the same to just calculate distWidth = value - hGaps and pass only this value to the _distributeWidth method.

fschinkel · 2024-05-22T12:09:51Z

eclipse-scout-core/src/layout/logicalgrid/LogicalGridLayoutInfo.ts

+   */
+  protected _distributeWidth<T>(cons: LogicalGridData, distWidth: number, spanWidth: number, widths: number[], fixedWidths: boolean[], calc: (equalWidth: number, width: number) => number) {
+    let hSpan = cons.gridw;
+    let equalWidth = Math.floor((distWidth + spanWidth) / hSpan);


This is not a proper way to distribute equally.
Consider the 99 distributed to 10 columns.
This will lead to 9 columns being 9 wide and the last one being 18 wide.

See pie charts or #222794 for a proper algorithm (ticket has a proof attached).

fschinkel · 2024-05-22T12:14:37Z

eclipse-scout-core/test/layout/logicalgrid/LogicalGridLayoutInfoSpec.ts

+      expect(rows[0][1]).toEqual(new Rectangle(75, 0, parentSize.width - 75, 30));
+    });
+
+    describe('maxWidth', () => {


add test cases for the problems pointed out in LogicalGridLayoutInfo

...rt.shared/src/main/java/org/eclipse/scout/rt/chart/shared/data/basic/chart/IChartConfig.java

eclipse-scout-core/src/testing/JasmineScoutUtil.ts

fschinkel · 2024-05-22T12:58:02Z

...nt/src/main/java/org/eclipse/scout/rt/client/ui/desktop/hybrid/AbstractFormHybridAction.java

+        DO_ENTITY result = exportResult(form);
+        if (result == null) {
+          result = createEmptyResult();
+        }
        fireHybridWidgetEvent("save", result);
      }
      else if (FormEvent.TYPE_RESET_COMPLETE == e.getType()) {


Result needs to be exported and sent to the UI in the reset case as well.
See HybridManager._onHybridFormEvent. The sent data will be set onto the Form and then a reset-Event is triggered. Therefore, the current data (after reset) needs to be sent otherwise it will be empty in the UI after a reset.

How did it work before my change? Nothing was done with the result. Does it need to be passed to fireHybridWidgetEvent?

bschwarzent · 2024-05-22T09:25:05Z

eclipse-scout-core/src/tile/TileGrid.ts

@@ -1266,7 +1266,7 @@ export class TileGrid<TTile extends Tile = Tile> extends Widget implements TileG
  findTileIndexAt(x: number, y: number, startIndex?: number, reverse?: boolean): number {


bschwarzent · 2024-05-22T10:46:00Z

eclipse-scout-core/src/tile/TileGridSelectionHandler.ts

+   * If there are tiles selected but there is no focused tile, an object is returned containing the tile to be focused.
+   * This is the last tile if diff is > 0 and the first tile otherwise.
+   */
+  protected _computeFocusedTileOrSelection(diff: number): TileGridSelectionInstruction {


I find it a bit confusing that the first part of this method looks exactly like the implementation of computeFocusedTile(). Maybe this could be combined by passing the array of tiles as an argument to computeFocusedTile()?

Why does the method use the word "or", but the implementation returns both the focused tile "and" the selection?

If all other computeXYZ() methods are public, shouldn't this method be public as well?

bschwarzent · 2024-05-22T10:50:01Z

eclipse-scout-core/src/tile/TileGridSelectionHandler.ts

-    let focusedTile = null;
-    let focusedTileIndex = -1;
-    let result = this._computeFocusedTile(xDiff);
+    let result = this._computeFocusedTileOrSelection(xDiff);
    if (result.selectedTiles !== null) {


Suggested change

if (result.selectedTiles !== null) {

if (result.selectedTiles) {

Or should we even need to check the length of the array? Maybe a JsDoc on the interface would help clarify the API.

Same with computeSelectionY.

org.eclipse.scout.rt.client/src/main/java/org/eclipse/scout/rt/client/ui/tile/AbstractTile.java

bschwarzent · 2024-05-22T11:27:28Z

org.eclipse.scout.rt.client/src/main/java/org/eclipse/scout/rt/client/ui/tile/AbstractTile.java

+  @Override
+  public void cancelLoading() {
+    if (m_loadJob != null) {
+      m_loadJob.cancel(true);


Maybe set the member to null? Or add .whenDone() when scheduling the job and set it to null, so it will also be reset upon normal completion.

bschwarzent · 2024-05-22T14:41:06Z

eclipse-scout-core/src/tile/TileGrid.ts

@@ -61,6 +62,8 @@ export class TileGrid<TTile extends Tile = Tile> extends Widget implements TileG
  updateTextFilterText: string;
  defaultMenuTypes: string[];
  wrappable: boolean;
+  movableProducer: (tile: Tile) => TileMoveHandler;
+  resizableProducer: (tile: Tile) => Resizable;


Same issue as with movableProducer.

bschwarzent · 2024-05-22T14:42:41Z

eclipse-scout-core/src/resizable/Resizable.ts

+  minBounds: Rectangle;
+  maxBounds: Rectangle;
+  distance: number[];
+  edge: string;


Consider using a string union type: edge: 'e' | 'n' | ...;

bschwarzent · 2024-05-22T14:45:37Z

eclipse-scout-core/src/resizable/Resizable.ts

+    if (this._context.currentBounds.equals(this._context.initialBounds)) {
+      return;
+    }
+    this.$container.trigger('resizeend', {


Is the spelling here intentional? It does not seem to be a native event, and I could not find any other reference to "resizeend" in the code.

bschwarzent · 2024-05-22T14:50:06Z

eclipse-scout-core/src/resizable/Resizable.ts

-      distance = this._calcDistance(ctx.mousedownEvent, event);
+    let newBounds = this._computeBounds(event);
+    if (newBounds) {
+      $.throttle(this._resizeHandler, Resizable.FPS)(newBounds);


Does that work? I think $.throttle returns a new function that wraps the original function. If $.throttle is called during each mouse movement, new wrappers are created, but the subsequent calls are not throttled.

bschwarzent · 2024-05-22T14:54:41Z

eclipse-scout-core/src/tile/TileGrid.ts

@@ -294,6 +324,16 @@ export class TileGrid<TTile extends Tile = Tile> extends Widget implements TileG
    this.setTiles([]);
  }

+  moveTileBefore(tileToMove: ObjectOrModel<TTile>, sibling: TTile) {
+    let tiles = arrays.moveBefore(this._tiles, tileToMove as Tile, sibling);


Why does the type of the first argument allow models? this._tiles does not contain any models (according to its type), therefore a model could never be moved.

cguglielmo marked this pull request as draft April 25, 2024 10:38

cguglielmo force-pushed the features/cgu/24.2/isd branch from 5f85d49 to 32a52ef Compare May 3, 2024 14:15

nsteger force-pushed the features/cgu/24.2/isd branch from 32a52ef to bb154e5 Compare May 7, 2024 10:20

cguglielmo force-pushed the features/cgu/24.2/isd branch 2 times, most recently from aeb7ec7 to e0acb81 Compare May 14, 2024 12:54

cguglielmo and others added 13 commits May 20, 2024 22:46

FormFieldTile: ensure tile widget is initialized even if set later

2ce6a08

Also, don't fail if there is no parent, because tiles may be created without one (e.g. by hybrid manager for a scout js tile grid)

AbstractFormHybridAction: extract add listener logic to allow overriding

b106478

objects: slightly improve objects.equalsRecursive

2afbaa7

The method already works correctly. The new early === check may be a tiny bit faster. The other changes are just code style related.

LogicalGridLayout: add spy to visualize grid

2ad0c2f

LogicalGridLayout: add support for cell max width-/height

5c2fe13

UserAgent: support IEnum for deviceType, engineType, layer, system

9934f41

This enables automatic serialization / deserialization if the enum is used in a data object.

StringField should implement StringFieldModel

a6aae3c

sizes.les: add iphone-height variable

ffde3cd

Add IChartConfig options constant, as provided by chartjs

888149f

jquery-scout: fix scrollParents

8369a84

The returned jquery collection contained jquery objects instead of html elements which is different to jquery collection by jquery and prevents using methods like .on()

Form: add computeSaveNeeded method

c38fc09

Makes it possible to add custom save needed behavior. Also removed saveNeeded from model because it is a computed property

cguglielmo force-pushed the features/cgu/24.2/isd branch from 60ed66c to dbe6c31 Compare May 20, 2024 20:57

cguglielmo added 11 commits May 21, 2024 16:35

TileGrid: add dnd support to move tiles

e33f8f2

TileGrid: add resize support

37375dd

Resizable: consider scroll position

1a2edf1

If the resizable element is inside a scroll container, the scroll position needs to be considered when drawing the overlay

FormFieldTile: use the tile selection color for the resizable overlay

50323fc

This ensures the resizable is always visible even on inverted tiles.

FormFieldTile: adjust resizable color

3a98e10

Jasmine: some cleanup

f3f6931

Remove obsolete namespace in spec name. Remove obsolete jshint annotation. Remove DEFAULT_TIMEOUT_INTERVAL because it will affect every spec not only FocusManagerSpec and it does not seem to be needed.

SplitBox: create enum for splitter position type

19ce4d2

SplitBox: add css class invisible-splitter

a7d53cf

This is useful if the splitter itself should be visualized in another way, e.g. by using one of the fields border.

HybridManager: fix type

564a9ab

HybridActionEvent uses actionType instead of eventType. Also move HybridEvent type to Adapter because it is only relevant for the communication with Java and not part of the hybrid manager api.

cguglielmo added 7 commits May 21, 2024 16:35

Jasmine: add utility to mock hybrid actions

a09d92b

Tile: add support to cancel and re-trigger data loading

0cd85e3

UserAgentUtility: add method to simulate another device

776e813

FormHybridAction: provide another exportResult method

1d47263

Sometimes the result object is already available. In that case it would be cumbersome to map the values into the given result object. With the new method the result object can be returned directly.

CodeModel: implement DoEntity

8ffee04

Codes delivered by the CodeResource are DOs and can therefor contain a _type property

TileGrid: improve keystroke handling with tiles having different sizes

9d05a6f

If some of the tiles have h or w > 1 and others don't, the navigation between the tiles did not work properly.

cguglielmo force-pushed the features/cgu/24.2/isd branch from dbe6c31 to 9d05a6f Compare May 21, 2024 14:39

fschinkel requested review from mvilliger, fschinkel and bschwarzent and removed request for mvilliger May 22, 2024 07:26

mvilliger reviewed May 22, 2024

View reviewed changes

fschinkel requested changes May 22, 2024

View reviewed changes

bschwarzent reviewed May 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Features/cgu/24.2/isd #984

Features/cgu/24.2/isd #984

cguglielmo commented Apr 25, 2024

cguglielmo commented May 21, 2024

mvilliger May 22, 2024

mvilliger May 22, 2024

fschinkel May 22, 2024

cguglielmo May 24, 2024

fschinkel May 22, 2024

fschinkel May 22, 2024

fschinkel May 22, 2024

fschinkel May 22, 2024

fschinkel May 22, 2024

cguglielmo May 24, 2024

bschwarzent May 22, 2024

bschwarzent May 22, 2024

bschwarzent May 22, 2024

bschwarzent May 22, 2024

bschwarzent May 22, 2024

bschwarzent May 22, 2024

bschwarzent May 22, 2024

bschwarzent May 22, 2024

bschwarzent May 22, 2024

@@ @@ -56,4 +56,25 @@ public static UserAgent getCurrentUserAgent() { @@
                   }
                 }
+                /**

	for (let j = cons.gridx; j < cons.gridx + cons.gridw && j < this.cols; j++) {
	if (cons.gridx < this.cols) {

		@@ -1266,7 +1266,7 @@ export class TileGrid<TTile extends Tile = Tile> extends Widget implements TileG
		findTileIndexAt(x: number, y: number, startIndex?: number, reverse?: boolean): number {

	if (result.selectedTiles !== null) {
	if (result.selectedTiles) {

Features/cgu/24.2/isd #984

Are you sure you want to change the base?

Features/cgu/24.2/isd #984

Conversation

cguglielmo commented Apr 25, 2024

cguglielmo commented May 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment