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

pass base64 flag in coverUrl | Book.coverUrl() hardcoded with blob ? No Base 64 #1219

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aneesijaz
Copy link

Background

I am working on electron based application (an embedded express server in electron wrapper) and I am getting the blob url something like blob:localhost:3000/ ...

I just saw the code and looks like the createUrl method inside the coverUrl does not have second parameter and it defaults to blob always but I need base64 so I passed { replacements: "base64" } and a little change to recipe got it working ...

Problem

coverUrl() {
   return this.loaded.cover.then(() => {
    if (!this.cover) {
      return null;
    }
    if (this.archived) {
      return this.archive.createUrl(this.cover);
    } else {
      return this.cover;
    }
  });
}

Solution

Is there any issue passing the 2nd argument containing base64 flag ? like in the other versions ?
something like :

coverUrl() {
   return this.loaded.cover.then(() => {
    if (!this.cover) {
      return null;
    }
    if (this.archived) {
      return this.archive.createUrl(this.cover, {"base64": this.settings.replacements === "base64"});
    } else {
      return this.cover;
    }
  });
}

Note: There are some indentation changes showing up and I think Test cases might also have to be updated but I didn't ...
And I don't have any experience of contributing so if it fits in then good .. if not then Excuse me .. :)

@fchasen
Copy link
Contributor

fchasen commented Oct 21, 2021

Sorry for the delay reviewing, this looks great but would it be possible to update this without the eslint formatting changes?

@aneesijaz
Copy link
Author

Sorry for the delay reviewing, this looks great but would it be possible to update this without the eslint formatting changes?

sure why not ...
I will pull the latest and code and make the changes again without es-lint formatting and push it (or at least that's how I believe it is going to work because I am not much familiar with the opensource contribution and how it works.. let me know if there is any other way to do it .. )...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants