-
Notifications
You must be signed in to change notification settings - Fork 273
fix: 解决 TreeWalker 跳过 Text 节点导致漏翻译的问题 #132
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
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdated DOM traversal to include Text nodes in the TreeWalker and extended grabNode to handle Text instances by mapping them to their nearest translatable parent element. Class diagram for updated grabNode and TreeWalker usageclassDiagram
class TreeWalker {
+nextNode(): Node
}
class Text {
}
class Element {
+tagName: string
}
class grabNode {
+grabNode(node: Element | Text): Element | false
}
class findTranslatableParent {
+findTranslatableParent(node: Text): Element | null
}
TreeWalker --> Text
TreeWalker --> Element
grabNode --> Element
grabNode --> Text
grabNode --> findTranslatableParent
findTranslatableParent --> Element
findTranslatableParent --> Text
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes - here's some feedback:
- Filter out empty or whitespace-only Text nodes in the TreeWalker acceptNode to reduce unnecessary processing.
- Update grabNode’s parameter and return types from any to more precise Node and Element|false to improve type safety.
- After resolving a Text node to a parent element, re-apply the element tag filters before accepting it to avoid including non-translatable parents.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Filter out empty or whitespace-only Text nodes in the TreeWalker acceptNode to reduce unnecessary processing.
- Update grabNode’s parameter and return types from any to more precise Node and Element|false to improve type safety.
- After resolving a Text node to a parent element, re-apply the element tag filters before accepting it to avoid including non-translatable parents.
## Individual Comments
### Comment 1
<location> `entrypoints/main/dom.ts:36` </location>
<code_context>
+ NodeFilter.SHOW_ELEMENT | NodeFilter.SHOW_TEXT,
{
acceptNode: (node: Node): number => {
+ if (node instanceof Text) return NodeFilter.FILTER_ACCEPT;
+
if (!(node instanceof Element)) return NodeFilter.FILTER_SKIP;
</code_context>
<issue_to_address>
Accepting Text nodes may lead to duplicate or unexpected parent node selection.
Ensure grabNode prevents adding the same parent node multiple times when processing multiple child Text nodes. Consider if deduplication is required to avoid duplicate entries in the result array.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| NodeFilter.SHOW_ELEMENT | NodeFilter.SHOW_TEXT, | ||
| { | ||
| acceptNode: (node: Node): number => { | ||
| if (node instanceof Text) return NodeFilter.FILTER_ACCEPT; |
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.
suggestion (bug_risk): Accepting Text nodes may lead to duplicate or unexpected parent node selection.
Ensure grabNode prevents adding the same parent node multiple times when processing multiple child Text nodes. Consider if deduplication is required to avoid duplicate entries in the result array.
复现网页样例: https://medium.com/cinemania/message-undelivered-821873861de3