korbit-ai[bot] commented on code in PR #35041:
URL: https://github.com/apache/superset/pull/35041#discussion_r2326894331


##########
superset/utils/webdriver.py:
##########
@@ -188,7 +188,6 @@ def get_screenshot(  # pylint: disable=too-many-locals, 
too-many-statements
                     # chart containers didn't render
                     logger.debug("Wait for chart containers to draw at url: 
%s", url)
                     slice_container_locator = page.locator(".chart-container")
-                    slice_container_locator.first.wait_for()
                     for slice_container_elem in slice_container_locator.all():
                         slice_container_elem.wait_for()

Review Comment:
   ### Missing Chart Container Verification <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The code now waits for all chart containers without first verifying that at 
least one exists. This can cause the loop to execute zero times, bypassing 
necessary waiting for chart containers to render.
   
   
   ###### Why this matters
   If a dashboard is expected to have charts, they might not be fully rendered 
before taking the screenshot, resulting in blank or incomplete visualizations 
in the captured image.
   
   ###### Suggested change ∙ *Feature Preview*
   Add a verification step before the loop to ensure charts are properly 
handled when present:
   ```python
   slice_container_locator = page.locator(".chart-container")
   if slice_container_locator.count() > 0:  # Only wait for charts if they exist
       for slice_container_elem in slice_container_locator.all():
           slice_container_elem.wait_for()
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bfbc957f-2f8c-4360-839d-e05131fef651/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bfbc957f-2f8c-4360-839d-e05131fef651?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bfbc957f-2f8c-4360-839d-e05131fef651?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bfbc957f-2f8c-4360-839d-e05131fef651?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bfbc957f-2f8c-4360-839d-e05131fef651)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:39ab063a-4f7f-4714-8aa3-0ddb9976c305 -->
   
   
   [](39ab063a-4f7f-4714-8aa3-0ddb9976c305)



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to