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 multiple items for douban.com #1762

Merged
merged 4 commits into from Oct 11, 2018

Conversation

@JanusChoi
Copy link
Contributor

commented Oct 9, 2018

The original Xpath //div/a[contains(@onclick, "moreurl")] is not working and I replace it with //div[@class="title"]/a' and modified following code in order to import multiple items in doulist or search result pages.

JanusChoi added some commits Oct 9, 2018

Fix DoWeb for importing multiple items in page
The original Xpath ```//div/a[contains(@OnClick, "moreurl")]``` is not working and I replace it with ```//div[@Class="title"]/a'``` and modified following code in order to import multiple items in doulist or search result pages.
fix translator metadata issue
fix translator metadata issue
@zuphilip
Copy link
Collaborator

left a comment

This looks fine in general. Please look at the comments and let us know when you have changed those.

Show resolved Hide resolved Douban.js Outdated
Show resolved Hide resolved Douban.js Outdated
Douban.js Outdated
//while (title = titles.iterateNext()) {
//Zotero.debug(title);
//items[title.href] = title.textContent;
//}

This comment has been minimized.

Copy link
@zuphilip

zuphilip Oct 9, 2018

Collaborator

Delete these lines.

Show resolved Hide resolved Douban.js
@zuphilip

This comment has been minimized.

Copy link
Collaborator

commented Oct 9, 2018

Did you made sure that the test cases are still working as before? This is easy to test when you use Scaffold plugin within Zotero.

@zuphilip zuphilip changed the title Fix DoWeb for importing multiple items in page Fix multiple items for douban.com Oct 9, 2018

JanusChoi added some commits Oct 10, 2018

delete comments & debug code
delete comments & debug code
fix more details from the review
fix more details from the review
@JanusChoi

This comment has been minimized.

Copy link
Contributor Author

commented Oct 10, 2018

Did you made sure that the test cases are still working as before? This is easy to test when you use Scaffold plugin within Zotero.

Thanks for the review!

Yes, I've already tested two links from the test cases and they work fine in Scaffold.

@zuphilip zuphilip merged commit 21e60d6 into zotero:master Oct 11, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@zuphilip

This comment has been minimized.

Copy link
Collaborator

commented Oct 11, 2018

Thank you very much @JanusChoi ! Merged now 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.