feat(gui): show region layer on workflow editor

[aglinxinyuan]

This PR adds a region layer to the workflow editor. The region layer becomes visible after the user clicks the "Run" button on the workflow.

The region layer will be hidden by default, and the user can enable it in the layer dropdown.

Demo:

Fix issue #3904

[Xiao-zhen-Liu]

LGTM in general. Left some minor comments.

[Yicong-Huang]

This is such a nice feature!!! I hope we can use color to indicate region state in the future, e.g., orange means running, green means completed

[aglinxinyuan]

This is such a nice feature!!! I hope we can use color to indicate region state in the future, e.g., orange means running, green means completed

Yes, that's a planned follow-up PR.

[chenlica]

@aglinxinyuan Can we add test cases?

[aglinxinyuan]

@aglinxinyuan Can we add test cases?

We don't need to add new test cases for the following reason:

1. This PR doesn’t introduce any new components — the functionality it affects is already covered by existing test cases.

2. The main change in this PR is a visual update on the front end, and our current testing framework doesn’t support GUI testing. Even if we used a framework that can test GUIs, defining the “correctness” of a visual element would still be difficult. For example, if a region’s shape is 10 px larger than expected due to differences in library algorithms, it would still be considered valid.

[Yicong-Huang]

@aglinxinyuan Can we add test cases?

We don't need to add new test cases for the following reason:

1. This PR doesn’t introduce any new components — the functionality it affects is already covered by existing test cases.
2. The main change in this PR is a visual update on the front end, and our current testing framework doesn’t support GUI testing. Even if we used a framework that can test GUIs, defining the “correctness” of a visual element would still be difficult. For example, if a region’s shape is 10 px larger than expected due to differences in library algorithms, it would still be considered valid.

I disagree. There are new code, there better be new tests. Try use AI to generate tests? At least you can test if this new component can be initialized properly.

[aglinxinyuan]

@aglinxinyuan Can we add test cases?

We don't need to add new test cases for the following reason:

1. This PR doesn’t introduce any new components — the functionality it affects is already covered by existing test cases.

2. The main change in this PR is a visual update on the front end, and our current testing framework doesn’t support GUI testing. Even if we used a framework that can test GUIs, defining the “correctness” of a visual element would still be difficult. For example, if a region’s shape is 10 px larger than expected due to differences in library algorithms, it would still be considered valid.

I disagree. There are new code, there better be new tests. Try use AI to generate tests? At least you can test if this new component can be initialized properly.

We have quite a few frontend test cases written in the way you described. Honestly, i don't like this approach—because in doing so, we’re essentially adding test cases just for the sake of having test cases. If we can’t find a meaningful way to verify a component’s functionality, then we’re just creating tests that will never fail and can’t actually catch any bugs. If any new code automatically means a new component and test cases, then we should add test cases for the readme file and issue templates.

[Yicong-Huang]

First of all, we should start to add more tests. Secondly, those simple initiation tests can at least help us to know if a component can be initialized or not, which is at least helpful when we upgrade dependencies.

Anyway, we can blame the existing tests, they can be improved. but that's not a reason not to add test here.

[Yicong-Huang]

All I'm asking now is adding tests for source code, I think that's reasonable. Actually spark is also testing docs, but we are not there yet.

[aglinxinyuan]

First of all, we should start to add more tests. Secondly, those simple initiation tests can at least help us to know if a component can be initialized or not, which is at least helpful when we upgrade dependencies.

Anyway, we can blame the existing tests, they can be improved. but that's not a reason not to add test here.

We already have initiation tests and the other tests for the workflow-editor. I'm not adding any new component in this PR.

[Yicong-Huang]

First of all, we should start to add more tests. Secondly, those simple initiation tests can at least help us to know if a component can be initialized or not, which is at least helpful when we upgrade dependencies.

Anyway, we can blame the existing tests, they can be improved. but that's not a reason not to add test here.

We already have initiation tests and the other tests for the workflow-editor. I'm not adding any new component in this PR.

That's good. Can we add some tests against your new code?

[aglinxinyuan]

All I'm asking now is adding tests for source code, I think that's reasonable. Actually spark is also testing docs, but we are not there yet.

Spark is testing docs doesn't automatically mean it's absolutely correct. Also their docs is very different from readme. The point I want to make is that testing markdown files is quite difficult since rendering can vary.

[aglinxinyuan]

First of all, we should start to add more tests. Secondly, those simple initiation tests can at least help us to know if a component can be initialized or not, which is at least helpful when we upgrade dependencies.

Anyway, we can blame the existing tests, they can be improved. but that's not a reason not to add test here.

We already have initiation tests and the other tests for the workflow-editor. I'm not adding any new component in this PR.

That's good. Can we add some tests against your new code?

That's why did I mentioned my second point in my earlier comment. In this PR, I'm drawing some shape in html with a library. It's not easy to compare drawing in joints with our current test framework. Even it's possible, it's hard to define what's a correct drawing.

[chenlica]

@aglinxinyuan Can resolve the conversation so that we can merge it?

[aglinxinyuan]

@aglinxinyuan Can resolve the conversation so that we can merge it?

I cannot find any unresolved conversation on my end. There might be a bug on GitHub, I will try to figure it out…

[Xiao-zhen-Liu]

LGTM now.