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

Export: uses markdown preview dom. Supports diagrams #3317

Merged
merged 2 commits into from
Dec 23, 2019

Conversation

shmolf
Copy link
Contributor

@shmolf shmolf commented Oct 26, 2019

Description

Changed from re-rendering the Markdown (which doesn't render the SVGs from Mermaid), to referencing the Preview DOM that's already rendered, thereby incorporating the already produced SVGs.

Issue fixed

TestText.txt
TestPDF.pdf

HTML and MD files are not supported by Github, so am not able to attach.

Type of changes

  • πŸ”˜ Bug fix (Change that fixed an issue)
  • βšͺ Breaking change (Change that can cause existing functionality to change)
  • πŸ”˜ Improvement (Change that improves the code. Maybe performance or development improvement)
  • βšͺ Feature (Change that adds new functionality)
  • βšͺ Documentation change (Change that modifies documentation. Maybe typo fixes)

Checklist:

  • πŸ”˜ My code follows the project code style
  • βšͺ I have written test for my code and it has been tested
  • πŸ”˜ All existing tests have been passed
  • πŸ”˜ I have attached a screenshot/video to visualize my change if possible

IssueHunt Summary

Referenced issues

This pull request has been submitted to:


@ZeroX-DG ZeroX-DG added the awaiting review ❇️ Pull request is awaiting a review. label Oct 27, 2019
@Flexo013
Copy link
Contributor

Just wondering if this helps with issue #3202 as well?

@shmolf
Copy link
Contributor Author

shmolf commented Oct 27, 2019

I tried the following:

$$x = {-b \pm \sqrt{b^2-4ac} \over 2a}$$

Neither one turned out well.

MathPDF.pdf

MathTEXT.txt

@shmolf
Copy link
Contributor Author

shmolf commented Nov 5, 2019

@Flexo013 I've added a new commit to allow for properly showing Latex.

Sample PDF from branch
example.pdf

@shmolf
Copy link
Contributor Author

shmolf commented Nov 20, 2019

@Flexo013 Does this need any more changes before review?

@ZeroX-DG
Copy link
Member

This one looks ok to me. Will test this when I get home.

@shmolf
Copy link
Contributor Author

shmolf commented Nov 22, 2019

@ZeroX-DG, did you encounter any issues?

@ZeroX-DG
Copy link
Member

Somehow when I export to pdf the latex text still disappear. I'm on Linux Mint.

@ZeroX-DG
Copy link
Member

That's strange, I tried to include the latex font (it was missing when exported) then the pdf export start working again. Then I remove the font completely and it still work. Does it related to cache or something?

@shmolf
Copy link
Contributor Author

shmolf commented Nov 24, 2019

Possibly.

I'll setup a Linux VM, and test it out

@ZeroX-DG
Copy link
Member

Confirm it's a cache issue. When I tried to clear all application data, the bug appears again. Solution for this is to include the katex font when we export the pdf.

@ZeroX-DG ZeroX-DG added awaiting changes πŸ–ŠοΈ Pull request has been reviewed, but contributor needs to make changes. and removed awaiting review ❇️ Pull request is awaiting a review. labels Nov 26, 2019
@Flexo013
Copy link
Contributor

Flexo013 commented Dec 3, 2019

@shmolf Could you apply the fix that @ZeroX-DG mentioned above? That way we can include this feature as part of the next release.

@shmolf
Copy link
Contributor Author

shmolf commented Dec 4, 2019

Sorry for the delay. Been racking my brain trying to think of a simpler solution but I think the real solution for the Katex font requires some significant code.

Issue layout:

All normal. All CSS references are properly exported, including the katex.css.
However, the katex.css itself is referencing the font families, which aren't ever handled by the export process.

/* stylelint-disable font-family-no-missing-generic-family-keyword */
@font-face {
  font-family: 'KaTeX_AMS';
  src: url(fonts/KaTeX_AMS-Regular.woff2) format('woff2'), url(fonts/KaTeX_AMS-Regular.woff) format('woff'), url(fonts/KaTeX_AMS-Regular.ttf) format('truetype');
  font-weight: normal;
  font-style: normal;
}

Other exporters, like the PDF, locally render the HTML, will pull any sub-referenced assets like the font files.

Solution: I think a new ticket should be created to parse through all sub-references to load the assets for the export. I don't know how/which node modules might carry a CSS file that's importing another CSS file, that may be importing yet another CSS file.

And I can't seem to get webcontents' savePage to work either. It works, but doesn't include the fonts.

  handleSaveAsHtml () {
    this.exportAsDocument('html', (noteContent, exportTasks, targetDir) => {
      const browser = new remote.BrowserWindow({show: false, webPreferences: {webSecurity: false, javascript: false}})
      browser.loadURL('data:text/html;charset=UTF-8,' + this.htmlContentFormatter(noteContent, exportTasks, targetDir))
      browser.webContents.on('did-finish-load', () => {
        browser.webContents.savePage(targetDir, 'HTMLComplete', () => {
          browser.destroy()
        })
      })
    })
  }

@shmolf
Copy link
Contributor Author

shmolf commented Dec 4, 2019

I propose removing the Katex issues from the Issue fixed list, and continuing to merge this in for the Diagram's sake.
And trying to resolve Katex fonts separately.

@ZeroX-DG
Copy link
Member

ZeroX-DG commented Dec 7, 2019

@shmolf I agree to separate the katex problem into a separate PR. I think the fix is just exporting the fonts to css/fonts/ folder in the same directory of the exported site (try export HTML and check console). But there maybe more to it.

@ZeroX-DG ZeroX-DG added approved πŸ‘ Pull request has been approved by sufficient reviewers. and removed awaiting changes πŸ–ŠοΈ Pull request has been reviewed, but contributor needs to make changes. labels Dec 7, 2019
@facorazza
Copy link

@shmolf I agree to separate the katex problem into a separate PR. I think the fix is just exporting the fonts to css/fonts/ folder in the same directory of the exported site (try export HTML and check console). But there maybe more to it.

Where can I find the fonts dir?

@ZeroX-DG
Copy link
Member

ZeroX-DG commented Dec 8, 2019

@Imperator26 When exporting a note, Boostnote will export CSS to a folder called css. Now you need to export the fonts to the fonts folder inside of the css folder and it should fix the problem

@facorazza
Copy link

@Imperator26 When exporting a note, Boostnote will export CSS to a folder called css. Now you need to export the fonts to the fonts folder inside of the css folder and it should fix the problem

Where can I find the fonts folder?

@ZeroX-DG
Copy link
Member

ZeroX-DG commented Dec 9, 2019

@Imperator26 you should support Boostnote to create it, because Boostnote currently doesn't export it automatically.

@shmolf
Copy link
Contributor Author

shmolf commented Dec 12, 2019

@Rokt33r Rokt33r merged commit ce853a7 into BoostIO:master Dec 23, 2019
@Rokt33r Rokt33r added this to the v0.14.0 milestone Dec 23, 2019
@Rokt33r Rokt33r removed the approved πŸ‘ Pull request has been approved by sufficient reviewers. label Dec 23, 2019
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.

None yet

5 participants