Fix issues for api.get.visible_emails #421
Conversation
'visible_emails' always returns the first 120 emails. This patch changes 2 string query params in the URL of the gmail request to get only visible emails.
This new method returns the number of emails shown per page.
|
Hey there and thanks for the PR! All contributions are welcome and very much appreciated. While this does look mostly reasonable, I have a few things I think may need to be addressed. I'll review things as best I can, but I'm always busy and can't make any promises on timeliness :) |
| Returns the number of emails shown per page | ||
| */ | ||
| emails_per_page(): number; | ||
| /** |
There was a problem hiding this comment.
Just pointing out that I really appreciate you updating the typescript references. That's great! :)
| url += "&start=" + start + | ||
| "&sstart=" + start; | ||
| url += "&start=" + parseInt(start - 1); | ||
| } else { |
There was a problem hiding this comment.
According to this comment on this older issue the sstart parameter is actually needed.
Was it causing you troubles? Are there any particular reason you decided to remove it?
| var start = $(".aqK:visible .Dj").find("span:first").text().replace(",", "").replace(".", ""); | ||
| var url = window.location.origin + window.location.pathname + "?ui=2&ik=" + api.tracker.ik+"&rid=" + api.tracker.rid + "&view=tl&rt=1&num=" + api.get.emails_per_page(); | ||
| var start = $(".aqK:visible .Dj").find("span.ts:first").text().replace(",", "").replace(".", ""); | ||
| if (start) { |
There was a problem hiding this comment.
I know we're already pretty heavy on magic CSS-selectors, but you could just for the sake of review outline how this .ts selector changes the behaviour of the function in a way which is consistent with your other changes?
Is this a required change, or is this a bugfix for something else which just happened to get submitted in the same batch?
|
@hitbits: If you want to get this PR merged, I would appreciate at least some feedback on the feedback I've given after taking the time to review your changes. I'm not going to move this forward until we have these things addressed. |
|
No response months, no other people reporting issues, and the underlying code has been changed in other ways since this PR was filed. Closing. |
2 participants
'visible_emails' always returns the first 120 emails. This patch changes
2 string query params in the URL of the gmail request to get only
visible emails.