Skip to content

Conversation

@IvoSG
Copy link
Contributor

@IvoSG IvoSG commented Oct 2, 2025

Feat for #3479.

  • Store viewName in the fragment body
  • Add additional info in other xml fragment templates

@IvoSG IvoSG requested review from a team as code owners October 2, 2025 14:50
@changeset-bot
Copy link

changeset-bot bot commented Oct 2, 2025

🦋 Changeset detected

Latest commit: 225b206

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
@sap-ux-private/preview-middleware-client Patch
@sap-ux/adp-tooling Patch
@sap-ux/preview-middleware Patch
@sap-ux/adp-flp-config-sub-generator Patch
@sap-ux/create Patch
@sap-ux/flp-config-inquirer Patch
@sap-ux/generator-adp Patch
@sap-ux/flp-config-sub-generator Patch

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

@nikmace nikmace added feature New feature or request preview-middleware @sap-ux/preview-middleware adp-tooling preview-middleware-client labels Oct 7, 2025
nikmace
nikmace previously approved these changes Oct 7, 2025
Copy link
Contributor

@nikmace nikmace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good
Changeset looks good
Coverage is good
Did not test manually

Please don't forget to fill the right panel of the PR with the assignee and labels 👍🏻

Comment on lines 31 to 42
if (templateName || (controlType && targetAggregation && viewName)) {
const result: AddXMLAdditionalInfo = {};
if (templateName) {
result.templateName = templateName;
}
if (controlType && targetAggregation && viewName) {
result.targetAggregation = targetAggregation;
result.controlType = controlType;
result.viewName = viewName;
}
return result;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to scale well. Next time we need to add something else we'll need to add another set of conditions which are repeated in two places. Maybe we can just check if there are any keys set before we return?

return Object.keys(result).length > 0 ? result : undefined;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed in: 5053c37

Comment on lines 97 to 99
if (isA('sap.ui.core.mvc.View', control)) {
return control as View;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (isA('sap.ui.core.mvc.View', control)) {
return control as View;
}
if (isA<View>('sap.ui.core.mvc.View', control)) {
return control;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed in: 5053c37

@IvoSG IvoSG requested a review from a team as a code owner October 21, 2025 12:18
@IvoSG IvoSG force-pushed the feat/3479/sotre-view-name-in-fragment branch from 017546c to 82ccbb4 Compare October 21, 2025 13:18
@IvoSG IvoSG force-pushed the feat/3479/sotre-view-name-in-fragment branch from 82ccbb4 to 225b206 Compare October 21, 2025 13:26
@sonarqubecloud
Copy link

@nikmace nikmace self-requested a review October 28, 2025 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adp-tooling feature New feature or request preview-middleware @sap-ux/preview-middleware preview-middleware-client

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants