Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat: The timing of thedoneEach hook call for the cover#2427

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
YiiGuxing wants to merge13 commits intodocsifyjs:develop
base:develop
Choose a base branch
Loading
fromYiiGuxing:feat-cover-lifecycle

Conversation

@YiiGuxing
Copy link

@YiiGuxingYiiGuxing commentedMay 13, 2024•
edited
Loading

Summary

ThedoneEach hook function for the cover should be called after the cover page HTML has been appended to the DOM.

Related issue, if any:

What kind of change does this PR introduce?

Feature

For any code change,

  • Related documentation has been updated, if needed
  • Related tests have been added or updated, if needed

Does this PR introduce a breaking change?

No

Tested in the following browsers:

  • Chrome
  • Firefox
  • Safari
  • Edge

The `doneEach` hook function for the cover should be called after the cover page HTML has been appended to the DOM.
@vercel
Copy link

vercelbot commentedMay 13, 2024•
edited
Loading

The latest updates on your projects. Learn more aboutVercel for Git ↗︎

NameStatusPreviewCommentsUpdated (UTC)
docsify-preview✅ Ready (Inspect)Visit Preview💬Add feedbackJun 5, 2024 2:17pm

@jhildenbiddle
Copy link
Member

Thanks of this,@YiiGuxing. Can you add some tests totest/e2e/plugins.test.js to verify the new behavior? Thx!

@YiiGuxing
Copy link
Author

@jhildenbiddle. Completed. Please review.

@YiiGuxing
Copy link
Author

@jhildenbiddle, I found that this PR does not work in the asynchronous mode ofmarked.js. I am fixing it, please wait for my patch.

jhildenbiddle reacted with thumbs up emoji

@YiiGuxing
Copy link
Author

@jhildenbiddle, The fact that the cover doesn't work in asynchronous mode onmarked.js isn't due to this PR, it's the same problem on thedevelop branch: settingmarkdown: { async: true } will cause the cover and the sidebar to malfunction. The new commit ensures that the cover works properly, but there are still issues with the sidebar. The new commits also add support for embedding files in the cover and fix an issue where thedoneEach hook might not be called.

Additionally, regarding thebeforeEach andafterEach hooks for the cover, these hooks are called twice when the cover and the homepage are on the same page. Since we cannot intuitively determine the target of the hook within the hook function, I have not implemented them.

@trusktr
Copy link
Member

I think we can merge this without having to fix the async stuff if that is not ready. It's a step forward, and we have to fix async at some point anyway

Copy link
Member

@trusktrtrusktr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I believe this can go in without async marked support. It would still be an upgrade, and we can make sure to document in which cases async marked mode currently does not work.

@YiiGuxing would you mind adding docs for marked async mode to make this clear? Then we'll be a step closer.

I need to also review the code, but generally I was also annoyed that I couldn't plug into all markdown rendering of the site and only the main content.

Ideally a plugin should be able to handle any section of markdown (navbar, sidebar, cover, main, etc) so we should definitely go in this direction.

@YiiGuxing
Copy link
Author

@trusktr Theoretically all cases compiled directly byCompiler.compile withoutembed.prerenderEmbed preprocessing will not work inmarked.js asynchronous mode, such assidebar,navigation andcover (of coursecover has addedembed.prerenderEmbed preprocessing in this PR to support embedding external files). The reason for this problem is that our compiler does not provide support formarked.js asynchronous mode. In asynchronous mode, the product ofmarked.js during compilation would be aPromise that the compiler could not handle and an exception would occur.
I don't think this issue has anything to do with this PR, I just found it when I was implementing this PR, and I'm not sure where the docs you're talking about should be added appropriately. I think it would be more appropriate to open a new issue request to add support formarked.js asynchronous mode, which I will do if you agree.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@trusktrtrusktrtrusktr requested changes

@jhildenbiddlejhildenbiddleAwaiting requested review from jhildenbiddle

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

5.x

Development

Successfully merging this pull request may close these issues.

5 participants

@YiiGuxing@jhildenbiddle@trusktr@Koooooo-7@sy-records
[8]ページ先頭
©2009-2025 Movatter.jp