-
-
Notifications
You must be signed in to change notification settings - Fork 876
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
[WIP] Fix/image link #322
[WIP] Fix/image link #322
Conversation
Codecov Report
@@ Coverage Diff @@
## master #322 +/- ##
==========================================
- Coverage 81.04% 80.00% -1.05%
==========================================
Files 8 8
Lines 902 915 +13
==========================================
+ Hits 731 732 +1
- Misses 171 183 +12
Continue to review full report at Codecov.
|
@@ -32,10 +35,38 @@ InteractableElement parseInteractableElement( | |||
switch (element.localName) { | |||
case "a": | |||
interactableElement.href = element.attributes['href']; |
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.
Too many abstraction levels in the body of 1 method.
You should break it down to smaller method/func
Infact, the code should do like this ( i will assume that your code logic is correct)
if(hasNoChild(interactableElement)) {
underline(interactableElement);
} else {
underlineChildTextElements(interactableElement);
}
Then, the method underlineChildTextElements will take care the recurrsive search & apply the corresponding style to its children
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.
Too many abstraction levels in the body of 1 method.
You should break it down to smaller method/func
Infact, the code should do like this ( i will assume that your code logic is correct)
if(hasNoChild(interactableElement)) {
underline(interactableElement);
} else {
underlineChildTextElements(interactableElement);
}
Then, the method underlineChildTextElements will take care the recurrsive search & apply the corresponding style to its children
Alternative fix: #499 499 |
Closing due to #499 |
Fixes #312