fix: reuse component detection #3038
Open
+446
−319
Conversation
This file contains 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
🦋 Changeset detectedLatest commit: 20076a1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 |
voicis
reviewed
Mar 24, 2025
| @@ -19,6 +19,7 @@ import { ContextMenuService } from './context-menu-service'; | |||
|
|
|||
| export default function init( | |||
| rta: RuntimeAuthoring, | |||
| isReuseComponentChecker: IsReuseComponentApi, | |||
There was a problem hiding this comment.
This can be undefined if called directly from RTA.
There was a problem hiding this comment.
As discussed after the changes proposed below, this point is taken care of.
Fixed with: a9fa288
| * | ||
| * @returns boolean | ||
| */ | ||
| export function isHigherThanMinimalUi5Version( |
There was a problem hiding this comment.
I don't think we need to "duplicate" isLowerThanMinimalUi5Version function. You can just use negation if you need higher version.
There was a problem hiding this comment.
Fixed with: 848ceb5
| */ | ||
| export const getExtendControllerItemText = (overlay: ElementOverlay, isReuseComponentChecker: IsReuseComponentApi, isCloud: boolean) => { | ||
| if (isCloud && isReuseComponentChecker(overlay.getElement().getId())) { | ||
| return 'Extend With Controller (Unavailable due to control being a Reuse component)'; |
There was a problem hiding this comment.
Text should be maintained in messagebundle.properties.
| @@ -40,6 +40,7 @@ export interface QuickActionContext { | |||
| flexSettings: FlexSettings; | |||
| manifest: Manifest; | |||
| changeService: ChangeService; | |||
| isReuseComponent: (controlId: string) => boolean; | |||
There was a problem hiding this comment.
Actually looking at this now, it seems that this is a bit overly complex and not needed. We could just "import" the isReuseComponent in the quick action where it is required, since it no longer has a state.
| reuseComponentApi = (await import('sap/ui/rta/util/isReuseComponent')).default; | ||
| } | ||
|
|
||
| return function isReuseComponent(controlId: string): boolean { |
There was a problem hiding this comment.
There is no need to redefine this function every time.
lfindlaysap
requested changes
Mar 25, 2025
| @@ -42,6 +42,11 @@ ADP_SYNC_VIEWS_MESSAGE = Synchronous views are detected for this application. Co | |||
| ADP_REUSE_COMPONENTS_MESSAGE_TITLE = Reuse components detected | |||
There was a problem hiding this comment.
Suggested change
| ADP_REUSE_COMPONENTS_MESSAGE_TITLE = Reuse components detected | |
| ADP_REUSE_COMPONENTS_MESSAGE_TITLE = Reuse Components Detected |
| @@ -42,6 +42,11 @@ ADP_SYNC_VIEWS_MESSAGE = Synchronous views are detected for this application. Co | |||
| ADP_REUSE_COMPONENTS_MESSAGE_TITLE = Reuse components detected | |||
| ADP_REUSE_COMPONENTS_MESSAGE_DESCRIPTION = Reuse components are detected for some views in this application. Controller extensions, adding fragments and manifest changes are not supported for such views and will be disabled. | |||
There was a problem hiding this comment.
Suggested change
| ADP_REUSE_COMPONENTS_MESSAGE_DESCRIPTION = Reuse components are detected for some views in this application. Controller extensions, adding fragments and manifest changes are not supported for such views and will be disabled. | |
| ADP_REUSE_COMPONENTS_MESSAGE_DESCRIPTION = Reuse components are detected for some views in this application. Controller extensions, adding fragments, and manifest changes are not supported for such views and will be disabled. |
| @@ -42,6 +42,11 @@ ADP_SYNC_VIEWS_MESSAGE = Synchronous views are detected for this application. Co | |||
| ADP_REUSE_COMPONENTS_MESSAGE_TITLE = Reuse components detected | |||
| ADP_REUSE_COMPONENTS_MESSAGE_DESCRIPTION = Reuse components are detected for some views in this application. Controller extensions, adding fragments and manifest changes are not supported for such views and will be disabled. | |||
| ADP_QUICK_ACTION_DIALOG_OPEN_MESSAGE = This action is disabled because a dialog is already open | |||
There was a problem hiding this comment.
Suggested change
| ADP_QUICK_ACTION_DIALOG_OPEN_MESSAGE = This action is disabled because a dialog is already open | |
| ADP_QUICK_ACTION_DIALOG_OPEN_MESSAGE = This action is disabled because a dialog is already open. |
| @@ -42,6 +42,11 @@ ADP_SYNC_VIEWS_MESSAGE = Synchronous views are detected for this application. Co | |||
| ADP_REUSE_COMPONENTS_MESSAGE_TITLE = Reuse components detected | |||
| ADP_REUSE_COMPONENTS_MESSAGE_DESCRIPTION = Reuse components are detected for some views in this application. Controller extensions, adding fragments and manifest changes are not supported for such views and will be disabled. | |||
| ADP_QUICK_ACTION_DIALOG_OPEN_MESSAGE = This action is disabled because a dialog is already open | |||
| ADP_ADD_FRAGMENT_MENU_ITEM = Add: Fragment | |||
| ADP_ADD_FRAGMENT_MENU_ITEM_REUSE_COMPONENT = Add: Fragment (Unavailable due to control being a Reuse component) | |||
There was a problem hiding this comment.
Can you show me what this looks like? Same for the other unavailable texts?
| ADP_ADD_FRAGMENT_MENU_ITEM = Add: Fragment | ||
| ADP_ADD_FRAGMENT_MENU_ITEM_REUSE_COMPONENT = Add: Fragment (Unavailable due to control being a Reuse component) | ||
| ADP_ADD_FRAGMENT_MENU_ITEM_UNSTABLE_ID = Add: Fragment (Unavailable due to unstable ID of the control or its parent control) | ||
| ADP_ADD_CONTROLLER_EXTENSION_MENU_ITEM = Extend With Controller |
There was a problem hiding this comment.
Suggested change
| ADP_ADD_CONTROLLER_EXTENSION_MENU_ITEM = Extend With Controller | |
| ADP_ADD_CONTROLLER_EXTENSION_MENU_ITEM = Extend with Controller |
| ADP_ADD_FRAGMENT_MENU_ITEM_REUSE_COMPONENT = Add: Fragment (Unavailable due to control being a Reuse component) | ||
| ADP_ADD_FRAGMENT_MENU_ITEM_UNSTABLE_ID = Add: Fragment (Unavailable due to unstable ID of the control or its parent control) | ||
| ADP_ADD_CONTROLLER_EXTENSION_MENU_ITEM = Extend With Controller | ||
| ADP_ADD_CONTROLLER_EXTENSION_MENU_ITEM_REUSE_COMPONENT = Extend With Controller (Unavailable due to control being a Reuse component) |
There was a problem hiding this comment.
Suggested change
| ADP_ADD_CONTROLLER_EXTENSION_MENU_ITEM_REUSE_COMPONENT = Extend With Controller (Unavailable due to control being a Reuse component) | |
| ADP_ADD_CONTROLLER_EXTENSION_MENU_ITEM_REUSE_COMPONENT = Extend with Controller (Unavailable due to control being a Reuse component) |
| ADP_ADD_FRAGMENT_MENU_ITEM_REUSE_COMPONENT = Add: Fragment (Unavailable due to control being a Reuse component) | ||
| ADP_ADD_FRAGMENT_MENU_ITEM_UNSTABLE_ID = Add: Fragment (Unavailable due to unstable ID of the control or its parent control) | ||
| ADP_ADD_CONTROLLER_EXTENSION_MENU_ITEM = Extend With Controller | ||
| ADP_ADD_CONTROLLER_EXTENSION_MENU_ITEM_REUSE_COMPONENT = Extend With Controller (Unavailable due to control being a Reuse component) | ||
| CPE_CHANGES_VISIBLE_AFTER_SAVE_AND_RELOAD_MESSAGE = Note: The change will be visible after save and reload. |
There was a problem hiding this comment.
Is there a button called save and reload?
| ADP_ADD_FRAGMENT_MENU_ITEM_REUSE_COMPONENT = Add: Fragment (Unavailable due to control being a Reuse component) | ||
| ADP_ADD_FRAGMENT_MENU_ITEM_UNSTABLE_ID = Add: Fragment (Unavailable due to unstable ID of the control or its parent control) | ||
| ADP_ADD_CONTROLLER_EXTENSION_MENU_ITEM = Extend With Controller | ||
| ADP_ADD_CONTROLLER_EXTENSION_MENU_ITEM_REUSE_COMPONENT = Extend With Controller (Unavailable due to control being a Reuse component) | ||
| CPE_CHANGES_VISIBLE_AFTER_SAVE_AND_RELOAD_MESSAGE = Note: The change will be visible after save and reload. | ||
|
|
||
| TABLE_ROWS_NEEDED_TO_CREATE_CUSTOM_COLUMN=At least one table row is required to create new custom column. Make sure the table data is loaded and try again. |
There was a problem hiding this comment.
Suggested change
| TABLE_ROWS_NEEDED_TO_CREATE_CUSTOM_COLUMN=At least one table row is required to create new custom column. Make sure the table data is loaded and try again. | |
| TABLE_ROWS_NEEDED_TO_CREATE_CUSTOM_COLUMN=At least one table row is required to create a new custom column. Make sure the table data is loaded and try again. |
GDamyanov
approved these changes
Mar 25, 2025
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
#3045
isReuseComponentAPI from RTA to detect reuse components.Add: FragmentandExtend with Controlleron control level only