[WIP] fix(DataMapper): Show a confirmation dialog when DataMapper step is t… #102
Conversation
| callback: (vizNode, modalAnswer) => { | ||
| metadataApi && onDeleteDataMapper(metadataApi, vizNode, modalAnswer); | ||
| }, | ||
| modalCustomization: { |
There was a problem hiding this comment.
Adding modal customization for the DataMapper step
| export interface IRegisteredInteractionAddon { | ||
| type: IInteractionAddonType; | ||
| activationFn: (vizNode: IVisualizationNode) => boolean; | ||
| callback: (vizNode: IVisualizationNode) => void; | ||
| callback: (vizNode: IVisualizationNode, modalAnswer: number | undefined) => void; | ||
| modalCustomization?: IModalCustomization; |
There was a problem hiding this comment.
Added modal customization option inside the IRegisteredInteractionAddon. For the DataMapper step specifically it is tightly coupled. The each callback execution needs to receive modalAnswer to determine if XSLT file is to be removed or not.
| DataMapperMetadataService.deleteMetadata(api, metadataId); | ||
| // TODO DataMapperMetadataService.deleteXsltFile(api, metadataId); | ||
| if (modalAnswer === ACTION_INDEX_DELETE_STEP_AND_FILE) { | ||
| await DataMapperMetadataService.deleteXsltFile(api, metadataId); |
There was a problem hiding this comment.
Here's exactly the one removing associated XSLT file for the target DataMapper step.
|
|
||
| let modalAnswer: number | undefined = 1; | ||
| if (props.loadActionConfirmationModal || modalCustoms.length > 0) { | ||
| const additionalModalText = modalCustoms.length > 0 ? modalCustoms[0].additionalText : undefined; |
There was a problem hiding this comment.
ATM the first found modal customization is used, but I wonder if there's any possibility to get more options in the future - say, if there's a sql step which has an external sql file, then we want to support removing the external sql file as well if user wants... but there's a DataMapper step right next to the sql step, and both are under when, and the user is deleting the when. How the modal should look like?
There was a problem hiding this comment.
I think it is possible to go that route if we want to, it will probably require checking what steps are involved first and then preparing the appropriate modal for that.
| const modalCustoms = findModalCustomizationRecursively(props.vizNode, (vn) => | ||
| getRegisteredInteractionAddons(IInteractionAddonType.ON_DELETE, vn), | ||
| ); | ||
| let modalAnswer: number | undefined = 1; |
There was a problem hiding this comment.
This is one of the reasons we should have used named indexes 😢 , it's not easy to reason about why 1.
There was a problem hiding this comment.
oh, but I see later on in the code:
import { ACTION_INDEX_DELETE_STEP_AND_FILE, ACTION_INDEX_DELETE_STEP_ONLY, MetadataContext } from '../../providers';can we use those? the benefit would be that when we refactor this functionality to use named indexes, it will be easier to know what's what.
There was a problem hiding this comment.
sure will do - we can use the string key in a same way
| getAddons: (vizNode: IVisualizationNode) => IRegisteredInteractionAddon[], | ||
| ) => { | ||
| const modalCustomizations: IModalCustomization[] = []; | ||
| // going breadth-first while addon processes depth-first... do we want? |
There was a problem hiding this comment.
I'm not sure if I understand the question, what do you mean? to pick between one or the other? If that's the case, I think yes, but I don't have any preference at the moment, feel free to pick the one that you consider the best.
There was a problem hiding this comment.
ATM there's no effect, this makes a difference once multiple modal customizations are introduced, on whether the sibling customization should come first or the deeper children should come first.
cf. #102 (comment)
packages/ui/src/components/registers/RegisterNodeInteractionAddons.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
@igarashitm feel free to move forward with this PR, if you want, I can give you a hand with the changes around the confirmation modal
|
The modal looks "interesting" with the cancel button trying to escape from the modal |
|
I'll fix to use string key for the button actions and modal button layout upstream first, put this PR on hold for a moment |
|
…o be deleted, with also offering an option to delete associated mapping file (XSLT) Fixes: https://github.com/KaotoIO/kaoto-datamapper-integration/issues/97
|
Merged upstream fix and verified 3 options works as expected |
…o be deleted, with also offering an option to delete associated mapping file (XSLT)
Fixes: KaotoIO/kaoto#1749
Here's what I could come up with. Let's start from here.
