Conversation
|
@luin @haroldtreen Please help review, and give some suggestions, thanks. |
|
This looks like an interesting option @hankliu62 👍 . What do you think about calling it Curious if @luin has thoughts because I know less about the history of this module 😅 . |
README.md
Outdated
| }); | ||
| ``` | ||
|
|
||
| - `candidatesFilters` which allow set your own filters for candidate tags. |
There was a problem hiding this comment.
This would read better as.
which allows you to set your own filters for candidate tags.
README.md
Outdated
| read(url, { | ||
| candidatesFilters: [ | ||
| function (obj) { | ||
| if (obj.tagName === 'ARTICLE' && elem.getAttribute('type') === 'video') { |
There was a problem hiding this comment.
Maybe could include a comment explaining what this filter does?
eg.
// ...
// Filter any article tags with a type of "video"
function(obj) {
// ...
}
README.md
Outdated
| ```javascript | ||
| read(url, { | ||
| candidatesFilters: [ | ||
| function (obj) { |
There was a problem hiding this comment.
obj doesn't describe what this function is passed. Maybe something like candidateNode?
README.md
Outdated
| read(url, { | ||
| candidatesFilters: [ | ||
| function (obj) { | ||
| if (obj.tagName === 'ARTICLE' && elem.getAttribute('type') === 'video') { |
There was a problem hiding this comment.
Also think elem is not defined in this example. I think you meant to make it the same as obj?
|
@haroldtreen, thanks for all your help! I've updated PR according to your instructions. Let me know if I've missed something. |
|
Thanks for the updates @hankliu62 ! Only last thing I would consider is bumping the version in package.json to |
No description provided.