Search panel headings #351
Conversation
lib/Page.js
Outdated
| this.collectHeadings($(panel).html()); | ||
| } | ||
| }); | ||
| this.collectHeadingsInRenderedPage(renderedPage); |
There was a problem hiding this comment.
The separation of the methods at this spot doesn't seem to make sense, because the context is broken. If the method is too long, then the panel logic should be the one getting refactored instead (i.e. collectHeadingsFromPanels()).
|
Currently there's a mismatch in behavior between this PR and MarkBind/vue-strap#80, will wait until MarkBind/vue-strap#80 is fully finished before reviewing resumes. |
|
I can't search for the heading "Bye" with the markdown below. Also, it seems that
<searchbar :data="searchData" placeholder="Search" :on-hit="searchCallback"></searchbar>
<panel header="# Hi" expanded>
<markdown>
## Bye
Bye
</markdown>
</panel> |
lib/Site.js
Outdated
| @@ -522,6 +522,11 @@ Site.prototype.generatePages = function () { | |||
| }); | |||
| return new Promise((resolve, reject) => { | |||
| Promise.all(processingFiles) | |||
| .then(() => { | |||
| this.pages.forEach((page) => { | |||
| page.collectHeadings(fs.readFileSync(page.resultPath)); | |||
There was a problem hiding this comment.
The test fails because page is a mock object rather than the an actual Page object, so page.resultPath is undefined (class attributes do not get mock).
Perhaps redesign the method such that it is unnecessary for Site.js to call fs.readFileSync(page.resultPath)?
lib/Site.js
Outdated
| @@ -522,6 +522,12 @@ Site.prototype.generatePages = function () { | |||
| }); | |||
| return new Promise((resolve, reject) => { | |||
| Promise.all(processingFiles) | |||
| .then((res) => { | |||
There was a problem hiding this comment.
The res doesn't seem to be necessary (the result is an undefined array, and is not used by subsequent code), so it can be removed (just .then(() => {)
lib/Page.js
Outdated
| if (panel.attribs.src) { | ||
| const includePath = path.resolve(this.resultPath, | ||
| path.relative(this.sourcePath, | ||
| path.relative(this.baseUrl, panel.attribs.src))); |
There was a problem hiding this comment.
There seems to be some implementation error computing the include path. I get this error:
Console:
warning: Error: ENOENT: no such file or directory, open 'C:\Users\other._include_.html'
warning: Error: ENOENT: no such file or directory, open 'C:\Users\other._include_.html'
error: ENOENT: no such file or directory, open 'C:\Users\other._include_.html'
index.md:
<panel header="Included panel" src="other.md" expanded>
</panel>other.md:
# Other Heading
Other heading
lib/Page.js
Outdated
|
|
||
| /** | ||
| * Records headings inside content into this.headings | ||
| * @param content that needs to collect headings |
There was a problem hiding this comment.
* @param content that contains the headings
lib/Page.js
Outdated
| @@ -165,21 +165,52 @@ Page.prototype.prepareTemplateData = function () { | |||
| }; | |||
|
|
|||
| /** | |||
| * Records h1,h2,h3 headings into this.headings | |||
| * @param renderedPage a page with its headings rendered | |||
| * Records headings in rendered page into this.headings | |||
There was a problem hiding this comment.
To be consistent with the below method's comment:
* Records headings inside rendered page into this.headings
lib/markbind/lib/parser.js
Outdated
| const resultDir = path.dirname(path.join('{{hostBaseUrl}}', path.relative(config.rootPath, src))); | ||
| const resultPath = path.join(resultDir, utils.setExtension(path.basename(src), '._include_.html')); | ||
| element.attribs.src = fragment ? `${resultPath}#${fragment}` : resultPath; | ||
| element.attribs.src = resultPath; |
There was a problem hiding this comment.
There is a bug in the rendering of panels with fragments after this line of code is changed. This suggests that the fragment is necessary after all:
There was a problem hiding this comment.
@nusjzx Btw this bug still exist, I presume you are not done with the PR, right? :P
452386e to
0a1d690
Compare
test/test_site/index.md
Outdated
| <panel header="## Panel without src" expanded> | ||
| <markdown> | ||
| ### panel without src content heading | ||
| <markdown> |
There was a problem hiding this comment.
Is this supposed to be the closing tag? </markdown>
test/test_site/index.md
Outdated
| @@ -38,4 +38,33 @@ siteNav: site-nav.md | |||
| # Include from another Markbind site | |||
| <include src="sub_site/index.md" /> | |||
|
|
|||
| # Panel without src | |||
| <panel header="## Panel without src" expanded> | |||
There was a problem hiding this comment.
The heading inside the panel is the same as the heading outside the panel. Maybe you can omit the heading outside the panel if it is the same as the one inside the panel?
This may be applicable for the ones below too.
90e2617 to
090622f
Compare
What is the purpose of this pull request? (put "X" next to an item, remove the rest)
• [ ] Documentation update
• [X] Bug fix
• [ ] New feature
• [ ] Enhancement to an existing feature
• [ ] Other, please explain:
Relates #336
What is the rationale for this request?
What changes did you make? (Give an overview)
collect headings from panels which are expanded.
Testing instructions:
1.pull MarkBind/vue-strap#80
2. run
npm run buildin vue-strap repo3. copy vue-strap.min.js to markbind repo
4. test on 2103 website.
eg: "pros": the pros and cons entry in the software engineer section are not indexed before(It's a panel header)
"abstraction" :
object as abstractionentry is there same as above.