Skip to content

refactor: Enhance XML Fragment context menu control with addXMLPlugin Integration #3091

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 57 commits into
base: main
Choose a base branch
from

Conversation

mmilko01
Copy link
Contributor

@mmilko01 mmilko01 commented Apr 7, 2025

#3093

  • For newer UI5 versions (>1.136) the RTA supplied addXMLPlugin is used.
  • The plugin provides the same functionality, but also gives better control on the context menu as it can be configured based on the designtime metadata of the UI element.

Copy link

changeset-bot bot commented Apr 7, 2025

🦋 Changeset detected

Latest commit: 3f6e999

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

This PR includes changesets to release 3 packages
Name Type
@sap-ux-private/preview-middleware-client Patch
@sap-ux/preview-middleware Patch
@sap-ux/create 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

@mmilko01 mmilko01 changed the title feat: Reuse addXMLPlugin feat: Reuse UI5 supplied addXMLPlugin in Adaptation Editor Apr 7, 2025
@mmilko01 mmilko01 changed the title feat: Reuse UI5 supplied addXMLPlugin in Adaptation Editor refactor: Enhance XML Fragment context menu control with addXMLPlugin Integration Apr 7, 2025
@mmilko01 mmilko01 marked this pull request as ready for review April 8, 2025 06:23
@mmilko01 mmilko01 requested review from a team as code owners April 8, 2025 06:23
public init() {
this.initPlugin();
CommunicationService.subscribe(async (action: ExternalAction): Promise<void> => {
if (addFragment.match(action)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably not needed anymore, as we have generic context menu in outline that works with action service.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in: 564535f

@@ -132,20 +133,25 @@ export const getExtendControllerItemText = (overlay: ElementOverlay, isReuseComp
* @param syncViewsIds Ids of all application sync views
* @param ui5VersionInfo UI5 version information
*/
export const initDialogs = async (rta: RuntimeAuthoring, syncViewsIds: string[], ui5VersionInfo: Ui5VersionInfo): Promise<void> => {
export const initDialogs = async (rta: RuntimeAuthoring, syncViewsIds: string[], ui5VersionInfo: Ui5VersionInfo): Promise<Promise<void>> => {
Copy link
Contributor

Choose a reason for hiding this comment

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

The return type does not look right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is a merging issue I missed.
Fixed in: bce4294

});
} else {
const addFragmentService = (await import('open/ux/preview/client/adp/add-fragment')).default;
new addFragmentService(rta).init();
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks a bit odd to create a new class instance but never actually use it. Wouldn't static methods or just initialisation function be more appropriate here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, refactored with static methods.
Fixed in: 48efacc

@mmilko01 mmilko01 requested a review from testojs April 9, 2025 09:05
@mmilko01 mmilko01 requested a review from a team as a code owner April 14, 2025 15:24
Copy link

Please retry analysis of this Pull-Request directly on SonarQube Cloud

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
49.1% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants