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

Mark scribblehub announcement-card as an author-note #1318

Merged
merged 4 commits into from
May 16, 2024

Conversation

nozwock
Copy link
Contributor

@nozwock nozwock commented May 15, 2024

In addition to the bottom author's note card, ScribbleHub also offers an announcement card (or news) option at the top.

This PR adds it to the Remove Author Note option.

Here's an instance of a chapter having it: https://www.scribblehub.com/read/426788-ecdysis/chapter/519798/

@dteviot
Copy link
Owner

dteviot commented May 16, 2024

@nozwock

Thanks for the commit.
However, it has the following minor issues.

  1. The actual change can be:
 this.tagAuthorNotesBySelector(webPageDom, ".wi_authornotes, .wi_news");
  1. You haven't added yourself to files readme.md or package.json, for credit.

Please fix, and squash, and I'll be delighted to accept.

also, if you want to change line 3 to

parserFactory.register("scribblehub.com", () => new ScribblehubParser());

feel free.

@dteviot
Copy link
Owner

dteviot commented May 16, 2024

@nozwock

And now that I look at it some more, this isn't used anymore.

    static nextTocPageUrl(baseUrl, nextTocIndex) {
        return `${baseUrl}?toc=${nextTocIndex}`;
    }

@nozwock
Copy link
Contributor Author

nozwock commented May 16, 2024

@dteviot
Done, you may squash and merge now.

Let me know if there's anything else.

@dteviot dteviot merged commit 7171ff0 into dteviot:master May 16, 2024
@dteviot
Copy link
Owner

dteviot commented May 16, 2024

@nozwock

Done. Thank you.

@nozwock nozwock deleted the exclude-scribblehub-announcement branch May 16, 2024 20:12
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