kie-issues#1636: DMN Editor throw an error opening Included Models tab

[Kusuma04-dev]

closes apache/incubator-kie-issues#1636 DMN Editor throw an error opening Included Models tab

[yesamer]

@Kusuma04-dev thank you for your PR!
Can you please share a screenshot of the change?
I mean, what is the impact of changing that Button to a div?

[Kusuma04-dev]

@Kusuma04-dev thank you for your PR! Can you please share a screenshot of the change? I mean, what is the impact of changing that Button to a div?

Before fix: There is a console error when we open included models which is telling button inside button shouldn't be there .

After fix:

[yesamer]

Thank you @Kusuma04-dev .
I suspect that Button element was added just for styling purposes.
If you notice, in the UI the Kebab Element has a different style with your changes.

Said so, your changes are correct from my point of view. if my above suspect is true, you can even totally remove that element, and not replace it with a div, unless we need that to stop the event propagation to the Kebab element. (to be checked)

My only doubt is about the element's style. Should we preserve it?

[jomarko]

I postpone my review until there is an answer for @yesamer comment.

[Kusuma04-dev]

Thank you @Kusuma04-dev . I suspect that Button element was added just for styling purposes. If you notice, in the UI the Kebab Element has a different style with your changes.

Said so, your changes are correct from my point of view. if my above suspect is true, you can even totally remove that element, and not replace it with a div, unless we need that to stop the event propagation to the Kebab element. (to be checked)

My only doubt is about the element's style. Should we preserve it?

Hi Yeser, Yeah i have kept div tag to stop event propogation ,also i don't see difference in the style ,as i have attached one ss with full screen and another with half screen , it might be looking different .

[yesamer]

@Kusuma04-dev Thank you for the clarification. The little difference I see in the style is on the Kebab component color: in the first screenshot it looks darker than the second one. However, it's just a minor difference, LGTM.

[ljmotta]

@Kusuma04-dev As @yesamer pointed it out, previously, the buttom was gray (not black) and changed color on hover. I don't think it's a problem to remove the hover if we don't have another solution for it.

previous-kebab.mp4

[Kusuma04-dev]

@Kusuma04-dev As @yesamer pointed it out, previously, the buttom was gray (not black) and changed color on hover. I don't think it's a problem to remove the hover if we don't have another solution for it.

previous-kebab.mp4

Hi Luiz, i have added css styles to make it look like before.

[jomarko]

Thank you for the PR. I can confirm there is no error in the browser console log 🥳 .

The only suggestion I have is to keep kie-tools css classes naming. We prefix custom classes in a way kie-[something like package]-- and then the new class is kie-[something like package]--[actual new name]. Maybe other colleagues will have better suggestion for new css names.

Good job!

[jomarko]

Thank you!