bamaer commented on PR #8636: URL: https://github.com/apache/hop/pull/8636#issuecomment-5854787508
Built `core,ui,rcp` at 6b40d62 with `-Puitest`: green, `GuiCompositeWidgetsGroupTest` 11/11, spotless clean. Then I measured the layout with a throwaway SWTBot test: 3 groups of 7/6/1 fields, 900x650 shell, `addScrolledComposite` (the recipe this PR's own docs prescribe), viewport 516px. | | main | this PR | GridLayout variant | |---|---|---|---| | content height | 678 | **1008** | 735 | | Connection (7 fields) | 300 | 318 | 319 | | Authentication (6 fields) | 273 | 309 | 292 | | Secrets (**1 field**) | 73 | **329** | 92 | ## Blocker: equal bands inflate the container to N x the tallest box A percentage band has no preferred height, so `filler.computeSize()` comes back as `count * max(box)`. Wherever the container sizes itself to its content -- `addScrolledComposite`, `ConfigPluginOptionsTab`, `AiProviderEditor` -- that is ~50% dead space plus an outer scrollbar that was not needed. The per-box scrolling never engages in that shape; the outer scrollbar is what moves. Dialogs affected today: Text chunker (3 boxes), the Vault resolver (3 boxes), the AI provider editor (2 groups + the models table). Suggested fix, measured in the table above: drop the percentage arithmetic, give `filler` a one-column `GridLayout` and each box `new GridData(SWT.FILL, SWT.FILL, true, true)`. The preferred height becomes the sum of the boxes' own heights, excess space is shared when the parent is taller, and a squeezed box still scrolls inside its own `ScrolledComposite`. ## Should fix - The `AiProviderEditor` hunk is the same problem seen from the other side: `Math.max(size.y, client.height)` is only needed because a band has no preferred height. With the layout fixed it should be able to go, rather than leaving a per-editor workaround behind. - `annotation-derived-widgets.adoc` line 238 refers to "the screenshots", but the page has none. Drop the phrase or add the images. - The same page documents "equal vertical bands", so that line has to follow whatever the layout ends up doing. - Worth stating in the docs that the composite handed to `createCompositeWidgets` has to be stretched (bottom at 100%) when `BOXES` is used, because a band contributes no preferred height. That is exactly the trap the `AiProviderEditor` change works around. ## Minor `boxesSplitTheParentIntoBands` asserts the `FormData` numerators, so it pins the implementation rather than the outcome and will need rewriting with any change to the band maths. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
