Add Page Building Block with Full Layout option during FPM app generation#4869
Open
kjose90 wants to merge 6 commits into
Open
Add Page Building Block with Full Layout option during FPM app generation#4869kjose90 wants to merge 6 commits into
kjose90 wants to merge 6 commits into
Conversation
🦋 Changeset detectedLatest commit: d456eb7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 36 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
…with full layout option during FPM app generation
kjose90
force-pushed
the
feat/add-page-template-support-app-gen
branch
from
7428cab to
4fa0510
Compare
Contributor
SummaryThe following content is AI-generated and provides a summary of the pull request: Add Page Building Block Full Layout Option During FPM App GenerationNew Features✨ Introduces a Full Layout option for the Page Building Block when generating FPM (Flexible Programming Model) custom page applications. Users can now choose between Basic (default, requires UI5 ≥ 1.136) and Full layout (requires UI5 ≥ 1.145) during app generation. Warning messages are shown for each layout option, and the minimum UI5 version requirement is automatically adjusted based on the selected layout. Changes
PR Bot InformationVersion:
|
![hyperspace-insights[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
The PR adds a well-structured feature with clear separation of concerns. Two issues were flagged: the additionalMessages callback in the layout question always shows a "Full layout" warning even before the user interacts with the prompt (missing undefined guard), and the full-layout version check in handlePageBuildingBlock silently allows full layout through when no UI5 version is specified (minVersion is null), bypassing the intended fallback to basic layout.
Comment on lines
+263
to
280
| additionalMessages: (input?: unknown) => { | ||
| // input is true when the user selects Basic layout (default/true = Basic) | ||
| if (input === true) { | ||
| return { | ||
| message: t('prompts.pageBuildingBlock.warning'), | ||
| message: t('prompts.pageBuildingBlock.basicLayoutWarning', { | ||
| minUi5VersionForPageBuildingBlock: MIN_UI5_VERSION_PAGE_BUILDING_BLOCK | ||
| }), | ||
| severity: Severity.warning | ||
| }; | ||
| } | ||
| // input is false when the user selects Full layout | ||
| return { | ||
| message: t('prompts.pageBuildingBlock.fullLayoutWarning', { | ||
| minUi5VersionForFullLayout: MIN_UI5_VERSION_PAGE_BUILDING_BLOCK_FULL_LAYOUT | ||
| }), | ||
| severity: Severity.warning | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
Logic Error: additionalMessages always returns a warning (even when input is undefined), meaning a "Full layout" warning is shown before the user has interacted with this prompt at all. When input is undefined (initial render, no answer yet), no message should be returned.
Suggested change
| additionalMessages: (input?: unknown) => { | |
| // input is true when the user selects Basic layout (default/true = Basic) | |
| if (input === true) { | |
| return { | |
| message: t('prompts.pageBuildingBlock.warning'), | |
| message: t('prompts.pageBuildingBlock.basicLayoutWarning', { | |
| minUi5VersionForPageBuildingBlock: MIN_UI5_VERSION_PAGE_BUILDING_BLOCK | |
| }), | |
| severity: Severity.warning | |
| }; | |
| } | |
| // input is false when the user selects Full layout | |
| return { | |
| message: t('prompts.pageBuildingBlock.fullLayoutWarning', { | |
| minUi5VersionForFullLayout: MIN_UI5_VERSION_PAGE_BUILDING_BLOCK_FULL_LAYOUT | |
| }), | |
| severity: Severity.warning | |
| }; | |
| } | |
| additionalMessages: (input?: unknown) => { | |
| // input is true when the user selects Basic layout (default/true = Basic) | |
| if (input === true) { | |
| return { | |
| message: t('prompts.pageBuildingBlock.basicLayoutWarning', { | |
| minUi5VersionForPageBuildingBlock: MIN_UI5_VERSION_PAGE_BUILDING_BLOCK | |
| }), | |
| severity: Severity.warning | |
| }; | |
| } | |
| // input is false when the user selects Full layout | |
| if (input === false) { | |
| return { | |
| message: t('prompts.pageBuildingBlock.fullLayoutWarning', { | |
| minUi5VersionForFullLayout: MIN_UI5_VERSION_PAGE_BUILDING_BLOCK_FULL_LAYOUT | |
| }), | |
| severity: Severity.warning | |
| }; | |
| } | |
| return undefined; | |
| } | |
Double-check suggestion before committing. Edit this comment for amendments.
Thank you for submitting your feedback: 🤷 Neutral.
Member
Author
There was a problem hiding this comment.
The prompt is of type confirm where default is set to true. So user at run time will either select true or false - so there is no scope for anything other than this.
But to make it clean - added an explicit check as suggested by bot for clarity.
Comment on lines
+130
to
+134
| if ( | ||
| templateType === PAGE_TEMPLATE_TYPE_FULL && | ||
| minVersion && | ||
| lt(minVersion.version, MIN_UI5_VERSION_PAGE_BUILDING_BLOCK_FULL_LAYOUT) | ||
| ) { |
Contributor
There was a problem hiding this comment.
Logic Error: When minVersion is null (i.e. data.minUI5Version is absent or unparseable), the full-layout version check is skipped entirely due to the minVersion && guard, so templateType stays as PAGE_TEMPLATE_TYPE_FULL and gets passed through without the version check. The fallback to PAGE_TEMPLATE_TYPE_BASIC and the warning should also apply when no version is specified, or at minimum the guard should be documented as intentional.
Suggested change
| if ( | |
| templateType === PAGE_TEMPLATE_TYPE_FULL && | |
| minVersion && | |
| lt(minVersion.version, MIN_UI5_VERSION_PAGE_BUILDING_BLOCK_FULL_LAYOUT) | |
| ) { | |
| if ( | |
| templateType === PAGE_TEMPLATE_TYPE_FULL && | |
| (!minVersion || lt(minVersion.version, MIN_UI5_VERSION_PAGE_BUILDING_BLOCK_FULL_LAYOUT)) | |
| ) { | |
| log?.warn( | |
| t('minUi5VersionRequirementFullLayout', { | |
| minUI5Version: data.minUI5Version, | |
| minUi5VersionForFullLayout: MIN_UI5_VERSION_PAGE_BUILDING_BLOCK_FULL_LAYOUT | |
| }) | |
| ); | |
| templateType = PAGE_TEMPLATE_TYPE_BASIC; | |
| } | |
Double-check suggestion before committing. Edit this comment for amendments.
Thank you for submitting your feedback: 🤷 Neutral.
lfindlaysap
requested changes
Jun 22, 2026
lfindlaysap
left a comment
lfindlaysap
left a comment
Contributor
There was a problem hiding this comment.
@kjose90, I did touch some adjacent lines as well. Hope that's okay.
| 'The Page building block feature requires SAPUI5 1.136.0 or higher. The current version is {{ minUI5Version }}, so the Page building block will not be added.' | ||
| 'The Page building block feature requires SAPUI5 {{ minUi5VersionForPageBuildingBlock }} or higher. The current version is {{ minUI5Version }}, so the Page building block will not be added.', | ||
| 'minUi5VersionRequirementFullLayout': | ||
| 'The Page building block full layout requires SAPUI5 {{ minUi5VersionForFullLayout }} or higher. The current version is {{ minUI5Version }}, so Basic layout will be applied instead.' |
Contributor
There was a problem hiding this comment.
Suggested change
| 'The Page building block full layout requires SAPUI5 {{ minUi5VersionForFullLayout }} or higher. The current version is {{ minUI5Version }}, so Basic layout will be applied instead.' | |
| 'Full Layout requires SAPUI5 {{ minUi5VersionForFullLayout }} or higher. The current version is {{ minUI5Version }}, so Basic Layout will be applied instead.' |
| "titleMessage": "Page Title" | ||
| "tooltip": "The Page building block is supported from SAPUI5 version {{ minUi5VersionForPageBuildingBlock }}. By choosing this option, you will be restricting the minimum SAPUI5 versions available for the application to be {{ minUi5VersionForPageBuildingBlock }} and above.", | ||
| "titleMessage": "Page Title", | ||
| "layoutMessage": "Choose Page Building Block layout", |
Contributor
There was a problem hiding this comment.
Suggested change
| "layoutMessage": "Choose Page Building Block layout", | |
| "layoutMessage": "Page Building Block Layout", |
| "tooltip": "The Page building block is supported from SAPUI5 version {{ minUi5VersionForPageBuildingBlock }}. By choosing this option, you will be restricting the minimum SAPUI5 versions available for the application to be {{ minUi5VersionForPageBuildingBlock }} and above.", | ||
| "titleMessage": "Page Title", | ||
| "layoutMessage": "Choose Page Building Block layout", | ||
| "choiceFull": "Full layout", |
Contributor
There was a problem hiding this comment.
Suggested change
| "choiceFull": "Full layout", | |
| "choiceFull": "Full Layout", |
| "titleMessage": "Page Title", | ||
| "layoutMessage": "Choose Page Building Block layout", | ||
| "choiceFull": "Full layout", | ||
| "choiceBasic": "Basic layout", |
Contributor
There was a problem hiding this comment.
Suggested change
| "choiceBasic": "Basic layout", | |
| "choiceBasic": "Basic Layout", |
| "choiceFull": "Full layout", | ||
| "choiceBasic": "Basic layout", | ||
| "basicLayoutWarning": "The Page building block is supported with SAPUI5 version {{ minUi5VersionForPageBuildingBlock }} and above.", | ||
| "fullLayoutWarning": "Full Page Layout is supported with SAPUI5 version {{ minUi5VersionForFullLayout }} and above." |
Contributor
There was a problem hiding this comment.
Suggested change
| "fullLayoutWarning": "Full Page Layout is supported with SAPUI5 version {{ minUi5VersionForFullLayout }} and above." | |
| "fullLayoutWarning": "Full Layout is supported with SAPUI5 version {{ minUi5VersionForFullLayout }} and above." |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.