Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix search on auto completion #1544

Merged
merged 1 commit into from
Jan 14, 2025
Merged

Conversation

LawyZheng
Copy link
Collaborator

@LawyZheng LawyZheng commented Jan 14, 2025

Important

Enhance auto-completion to support direct search and improve error handling and DOM visibility checks.

  • Behavior:
    • Update auto-completion-choose-option.j2 to include a third goal for direct searching if is_search is true.
    • Add direct_searching key in JSON response to indicate if direct search is preferable.
    • Modify choose_auto_completion_dropdown() in handler.py to handle direct_searching and press Enter if true.
  • Error Handling:
    • Add exception logging in handle_input_text_action() in handler.py.
    • Improve visibility checks in is_visible() in dom.py and domUtils.js.
  • DOM Handling:
    • Update domUtils.js to handle ul and div with role=listbox in mutation observer.
    • Add check for DOM navigation in stop_listen_dom_increment() in scraper.py.

This description was created by Ellipsis for 6643e7b. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Reviewed everything up to 6643e7b in 50 seconds

More details
  • Looked at 158 lines of code in 5 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. skyvern/webeye/scraper/scraper.py:579
  • Draft comment:
    Consider adding a more robust check to ensure the observer is properly initialized before checking if it's undefined.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code checks if the DOM has navigated away or refreshed before stopping the observer. This is a good practice to avoid errors, but the check is done using a JavaScript expression that evaluates if the observer is undefined. This could potentially lead to issues if the observer is not properly initialized or if there are other reasons for it being undefined. A more robust check might be needed.
2. skyvern/webeye/actions/handler.py:1702
  • Draft comment:
    Ensure that the visibility check does not introduce significant delays in the function execution.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code checks if the element is visible before clearing the input. This is a good practice to avoid errors when interacting with elements that might not be present or visible. However, the check is done using an asynchronous call, which might introduce a delay. It's important to ensure that this delay does not affect the overall performance of the function.
3. skyvern/webeye/actions/handler.py:793
  • Draft comment:
    Ensure that the visibility check does not introduce significant delays in the function execution.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code adds a check for the visibility of the element before triggering the input-selection hack. This is a good practice to ensure that the element is interactable before performing actions on it. However, the check is done using an asynchronous call, which might introduce a delay. It's important to ensure that this delay does not affect the overall performance of the function.
4. skyvern/webeye/scraper/domUtils.js:2134
  • Draft comment:
    Consider refactoring the node type check into a separate function to avoid code duplication.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code adds a check for the node type before processing mutations. This is a good practice to avoid unnecessary processing of text nodes, which are not interactable. However, the check is done multiple times in different parts of the code. It might be beneficial to refactor this into a separate function to avoid code duplication and improve maintainability.

Workflow ID: wflow_vmog1LX6Syl6k1lp


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@LawyZheng LawyZheng merged commit d63061f into main Jan 14, 2025
6 checks passed
@LawyZheng LawyZheng deleted the lawy/support-search-on-auto-completion branch January 14, 2025 05:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant