feat: The timing of the doneEach hook call for the cover
#2427
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -433,7 +433,6 @@ export function Render(Base) { | |
| { | ||
| compiler: /** @type {Compiler} */ (this.compiler), | ||
| raw: result, | ||
| }, | ||
| tokens => { | ||
| html = /** @type {Compiler} */ (this.compiler).compile(tokens); | ||
|
|
@@ -444,108 +443,71 @@ export function Render(Base) { | |
| }); | ||
| } | ||
|
|
||
| _renderCover(text, coverOnly, next) { | ||
| const el = /** @type {HTMLElement} */ (dom.getNode('.cover')); | ||
| const rootElm = document.documentElement; | ||
| // TODO this is now unused. What did we break? | ||
| // eslint-disable-next-line no-unused-vars | ||
| const coverBg = getComputedStyle(rootElm).getPropertyValue('--cover-bg'); | ||
|
|
||
| dom.getNode('main').classList[coverOnly ? 'add' : 'remove']('hidden'); | ||
|
|
||
| if (!text) { | ||
| el.classList.remove('show'); | ||
| next(); | ||
| return; | ||
| } | ||
|
|
||
| el.classList.add('show'); | ||
|
|
||
| const callback = html => { | ||
| const m = html | ||
| .trim() | ||
| .match('<p><img.*?data-origin="(.*?)"[^a]+alt="(.*?)">([^<]*?)</p>$'); | ||
|
|
||
| if (m) { | ||
| if (m[2] === 'color') { | ||
| el.style.background = m[1] + (m[3] || ''); | ||
| } else { | ||
| let path = m[1]; | ||
|
|
||
| el.classList.add('has-mask'); | ||
| if (!isAbsolutePath(m[1])) { | ||
| path = getPath(this.router.getBasePath(), m[1]); | ||
| } | ||
|
|
||
| el.style.backgroundImage = `url(${path})`; | ||
| el.style.backgroundSize = 'cover'; | ||
| el.style.backgroundPosition = 'center center'; | ||
| } | ||
|
|
||
| html = html.replace(m[0], ''); | ||
| } | ||
|
Member
There was a problem hiding this comment. Looks like this gradient background code is removed. Need to restore it. @YiiGuxing would you mind restoring the random gradient? |
||
|
|
||
| dom.setHTML('.cover-main', html); | ||
| next(); | ||
| }; | ||
|
|
||
| // TODO: Call the 'beforeEach' and 'afterEach' hooks. | ||
| // However, when the cover and the home page are on the same page, | ||
| // the 'beforeEach' and 'afterEach' hooks are called multiple times. | ||
| // It is difficult to determine the target of the hook within the | ||
| // hook functions. We might need to make some changes. | ||
|
Member
There was a problem hiding this comment. @YiiGuxing I think we should update the hooks in a backwards compatible way, such that the hook can somehow see which content it is called for. For example it can see string values like "cover", "sidebar", etc. The default (right now no sort of value to determine the target of the hook) would work the same as before for anyone who hasn't updated their plugin code.
Member
There was a problem hiding this comment. The ultimate result should be, that all markdown files are handled the same, regardless where in the app they are rendered (cover, sidebar, nav, etc) but any plugin can easily decide which to handle. Something like hook.doneEach((section) => {
if (section === 'cover') {
// run logic after cover rendered
} else if (section === 'sidebar') {
// run logic after sidebar rendered
} else if (section === 'nav') {
// run logic after nav bar rendered
} else if (section === 'main') {
// run logic after main content rendered (new way)
} else {
// run logic after main content rendered (old way, fallback)
}
})or similar. |
||
| if (this.coverIsHTML) { | ||
| callback(text); | ||
| } else { | ||
| const compiler = this.compiler; | ||
| if (!compiler) { | ||
| throw new Error('Compiler is not initialized'); | ||
| } | ||
| prerenderEmbed( | ||
| { | ||
| compiler, | ||
| raw: text, | ||
| }, | ||
| tokens => callback(compiler.cover(tokens)), | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| _updateRender() { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import docsifyInit from '../helpers/docsify-init.js'; | ||
| import { test, expect } from './fixtures/docsify-init-fixture.js'; | ||
|
|
||
| test.describe('Embed files', () => { | ||
| const routes = { | ||
| 'fragment.md': '## Fragment', | ||
| }; | ||
|
|
||
| test('embed into homepage', async ({ page }) => { | ||
| await docsifyInit({ | ||
| routes, | ||
| markdown: { | ||
| homepage: "# Hello World\n\n[fragment](fragment.md ':include')", | ||
| }, | ||
| // _logHTML: {}, | ||
| }); | ||
|
|
||
| await expect(page.locator('#main')).toContainText('Fragment'); | ||
| }); | ||
|
|
||
| test('embed into cover', async ({ page }) => { | ||
| await docsifyInit({ | ||
| routes, | ||
| markdown: { | ||
| homepage: '# Hello World', | ||
| coverpage: "# Cover\n\n[fragment](fragment.md ':include')", | ||
| }, | ||
| // _logHTML: {}, | ||
| }); | ||
|
|
||
| await expect(page.locator('.cover-main')).toContainText('Fragment'); | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This variable is now unused in the PR. Is the feature relating to that now deleted? If so, we need to fix that.
Also there are type errors I'm fixing (my recent changes added type checking to the code base).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that tests run, a jest test is failing. https://github.com/docsifyjs/docsify/actions/runs/19981379473/job/57308351783?pr=2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed the error, a method moved to the utils file.
Now the only thing failing is snapshots for the cover page. The diff shows this failure:
Looks like a couple classes are missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's the visual difference:
Before:
After (broken):
Maybe I missed something when merging in
develop?