-
Notifications
You must be signed in to change notification settings - Fork 417
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
iframes support #405
iframes support #405
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Reviewed everything up to 4ffca62 in 3 minutes and 11 seconds
More details
- Looked at
561
lines of code in8
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_qxTQtaWoCOkBVoBu
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
⌛ 4 days left in your free trial, upgrade for $20/seat/month or contact us.
skyvern/webeye/actions/actions.py
Outdated
@@ -96,7 +96,7 @@ class SelectOption(BaseModel): | |||
index: int | None | |||
|
|||
def __repr__(self) -> str: | |||
return f"SelectOption(label={self.label}, value={self.value}, index={self.index})" | |||
return f"SelectOption(label={self.label}, value={self.value}, index={self.index}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return f"SelectOption(label={self.label}, value={self.value}, index={self.index}" | |
return f"SelectOption(label={self.label}, value={self.value}, index={self.index})" |
There was a problem hiding this 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! Incremental review on 8bd4dc4 in 2 minutes and 2 seconds
More details
- Looked at
22
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. skyvern/webeye/scraper/domUtils.js:764
- Draft comment:
Ensure that the handling of iframe elements here aligns with the new iframe support features introduced in the PR. This change from single to double quotes is stylistic and does not impact functionality. - Reason this comment was not posted:
Confidence changes required:33%
The PR introduces iframe support, but the code snippet provided shows a change in the handling of iframe elements within thebuildTreeFromBody
function. The change is from using single quotes to double quotes for the string 'iframe'. This is a minor stylistic change and does not affect functionality. However, it's important to ensure that the handling of iframe elements is consistent with the new features introduced in the PR.
Workflow ID: wflow_xp4mYjjkbdYsQH5q
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
⌛ 4 days left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this 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! Incremental review on f39b2c4 in 2 minutes and 12 seconds
More details
- Looked at
13
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. skyvern/webeye/actions/actions.py:96
- Draft comment:
The PR description mentions updates to thebuildTreeFromBody
function indomUtils.js
for iframe support, but the provided diff only shows a change in this file. Please confirm if the changes related todomUtils.js
are included in another part of the PR or if the description needs updating. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_0eG13KONVbmoSFmd
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
⌛ 4 days left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this 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! Incremental review on 73a7d8d in 1 minute and 22 seconds
More details
- Looked at
325
lines of code in3
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. skyvern/webeye/actions/handler.py:979
- Draft comment:
Theresolve_locator
function may not correctly resolve locators for deeply nested iframes due to the order of frame traversal. Consider revising the logic to ensure that frames are accessed in the correct hierarchical order.
# Ensure correct frame traversal order for nested iframes
- Reason this comment was not posted:
Confidence of 10% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_GKztayynWpl5qTEN
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
⌛ 2 days left in your free trial, upgrade for $20/seat/month or contact us.
Summary:
This pull request enhances iframe support in web scraping and action handling, including specific updates to the
buildTreeFromBody
function indomUtils.js
.Key points:
VISIBLE_ELEMENTS_ID_FRAME_MAP
artifact type inArtifactType
enum.buildTreeFromBody
function indomUtils.js
.Generated with ❤️ by ellipsis.dev