add tests for the node resize feature and properties panel #27
Conversation
jomarko
force-pushed
the
kie-issues-946-part-3-resize
branch
from
ac293fe to
118c202
Compare
jomarko
changed the title
| <div data-testid="node-font-style-selector"> | ||
| <Select | ||
| toggleRef={toggleRef} | ||
| variant={SelectVariant.single} | ||
| aria-label={"Select font style"} | ||
| isOpen={isFontFamilySelectOpen} | ||
| onSelect={onSelectFont} | ||
| onToggle={() => setFontFamilySelectOpen((prev) => !prev)} | ||
| selections={fontFamily ?? ""} | ||
| isDisabled={false} | ||
| maxHeight={inViewTimezoneSelect.maxHeight} | ||
| direction={inViewTimezoneSelect.direction} | ||
| > | ||
| {WEBSAFE_FONTS_LIST.map((fontName, index) => ( | ||
| <SelectOption key={index} value={fontName} style={{ fontFamily: fontName }} /> | ||
| ))} | ||
| </Select> | ||
| </div> |
There was a problem hiding this comment.
We can not simply add data-testid int Select, patternfly for some reason ignores that attribute.
There was a problem hiding this comment.
I guess we could use a React.Fragment instead of the div? Not sure if the div changes something on the UI.
There was a problem hiding this comment.
Probably we can not, <React.Fragment data-testid={"xyz"}> has no effect for the produced html. The data-testid is not in the generated html at all.
jomarko
changed the title
There was a problem hiding this comment.
Alright, the tests are looking good, but it's still missing a few ones. Some of them I've added inline on the test files. We also need to test the documentation links with more cases. Add one more and change orders, for example. We also need to test the reset style button, for now, we can only test with the node font. The documentation links can be added to a different file, as it's not node specific.
Great job on the resize tests. But they are missing a few cases too:
- Resize with the snapping on?
- Change the snapping values?
- Resize on top of another node?
Now, regarding the fixtures, I don't think they're looking good. The class inheritance doesn't fit in some cases, making it necessary to add throw new Error methods. The way to solve this is with class composition. Instead of inheritance, we pass the instance or create the instance on the constructor and attribute it to one property. This way the class will have only the necessary methods. Also, I don't think this would be good either. We can't be sure the nodes will always share those properties, and maybe the best way is to replicate the logic on each class, removing the coupling altogether. Duplicating the code isn't a bad thing in this case, as it'll remove the complexity of creating the abstraction.
| <div data-testid="node-font-style-selector"> | ||
| <Select | ||
| toggleRef={toggleRef} | ||
| variant={SelectVariant.single} | ||
| aria-label={"Select font style"} | ||
| isOpen={isFontFamilySelectOpen} | ||
| onSelect={onSelectFont} | ||
| onToggle={() => setFontFamilySelectOpen((prev) => !prev)} | ||
| selections={fontFamily ?? ""} | ||
| isDisabled={false} | ||
| maxHeight={inViewTimezoneSelect.maxHeight} | ||
| direction={inViewTimezoneSelect.direction} | ||
| > | ||
| {WEBSAFE_FONTS_LIST.map((fontName, index) => ( | ||
| <SelectOption key={index} value={fontName} style={{ fontFamily: fontName }} /> | ||
| ))} | ||
| </Select> | ||
| </div> |
There was a problem hiding this comment.
I guess we could use a React.Fragment instead of the div? Not sure if the div changes something on the UI.
packages/dmn-editor/tests/e2e/drgElements/changeMultipleNodesProperties.spec.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Missing the skipped change color test.
There was a problem hiding this comment.
Sorry, I do not get it, as I tried to explain, I am not able to write a test for a color change using playwright. Could you please help me and advise how to write such a test? I am not able to handle the color picker we use in KIE tooling.
packages/dmn-editor/tests/e2e/drgElements/changeDiagramProperties.spec.ts
Outdated
Show resolved
Hide resolved
packages/dmn-editor/tests/e2e/drgElements/changeDiagramProperties.spec.ts
Outdated
Show resolved
Hide resolved
packages/dmn-editor/tests/e2e/drgElements/changeDecisionServiceProperties.spec.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Luiz João Motta <[email protected]>
Co-authored-by: Luiz João Motta <[email protected]>
Co-authored-by: Luiz João Motta <[email protected]>
* Add fill and stroke color change * Add assertion
…acts/resize-back-text-annotation-on-top-of-decision.png
jomarko
force-pushed
the
kie-issues-946-part-3-resize
branch
from
6ba7c02 to
af9a679
Compare
|
replaced by apache#2239 |
As part of this PR we add:
resize-handleelement we increase its size and then we check the size property in the properties panel