PG1204 commented on issue #5726:
URL: https://github.com/apache/texera/issues/5726#issuecomment-4997654554

   > Some context, since I've followed this since the original fix 
([#5146](https://github.com/apache/texera/pull/5146)):
   > 
   > The starting problem was that two separate parts of the code set an 
operator's border color and could overwrite each other. 
[#5146](https://github.com/apache/texera/pull/5146) fixed that by routing both 
through one function so the color is always right. 
[#5702](https://github.com/apache/texera/pull/5702) and 
[#6075](https://github.com/apache/texera/pull/6075) then made both paths get 
the validation result the same way.
   > 
   > What's left in [#5726](https://github.com/apache/texera/issues/5726) is 
the last small piece: when an operator is added, its border still gets painted 
by two paths instead of one. They produce the same color, so nothing looks 
wrong — it's just a redundant repaint.
   > 
   > How I'd finish it:
   > 
   > * In the operator-add handler, keep only the status restore 
(`changeOperatorStatistics`) and drop the extra `applyOperatorBorder` call. The 
validation pass runs at the same moment and can set the border color.
   > * Add a small guard so repainting the border with a color it already has 
does nothing. That also cleans up the navigation-return (reload) case.
   > 
   > This is safe for [#3614](https://github.com/apache/texera/issues/3614): 
the validation runs in the same step as the add, not later, so the border is 
never left blank waiting for it.
   > 
   > Manual testing is needed to confirm this, not just unit tests. Navigate 
away from a workflow and back and check that operator borders are restored 
correctly, including operators with a cached run status (completed/running) and 
invalid operators (which should stay red). A navigation-return unit test should 
be added on top of the manual check.
   > 
   > Happy to review whoever picks it up.
   
   @Xiao-zhen-Liu thanks for the input. I'll cover the two points mentioned in 
a fresh PR. Would you recommend creating a new issue and linking a new PR that 
would close both - the new issue and this current issue (#5726), or just link a 
PR to close this issue?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to