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

Feature/use url for convert #19

Merged

Conversation

edcohen08
Copy link
Collaborator

@edcohen08 edcohen08 commented May 25, 2022

Description

This uses the URL of the page for the bionic reading API request rather than doing a separate request for each text block for convertPage. The goal / motivation is to limit the number of API calls required assuming people will want to use the Basic plan. Motivation generally is that I really want to use this extension lol

Also secondary is that I'm working on another PR to handle a quota limit error from the api and it's easier to handle that if each type of conversion only makes one api request

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

@kwame-mintah
Copy link
Owner

kwame-mintah commented May 25, 2022

Just saw your PR, thanks for taking the time to do this.

Much appreciated @edcohen08

Will review shortly.

@kwame-mintah kwame-mintah self-requested a review May 25, 2022 12:30
@kwame-mintah
Copy link
Owner

I had a play with it on https://lipsum.com/, in comparison to what is in v0.1.0 - the page layout does change. However, looking at the RapidAPI dashboard, less calls are being made to the endpoint.

I am in favour of getting this merged in, can address the layout moving at a later date.

Had no idea you could request for a website to be converted, just by passing the URL, thanks for this @edcohen08 😅

@kwame-mintah kwame-mintah merged commit f6d15b9 into kwame-mintah:master May 25, 2022
@kwame-mintah kwame-mintah added the enhancement New feature or request label May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants