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

SEAB-5117: Display formatted notebook #1747

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
66 commits
Select commit Hold shift + click to select a range
0be8a50
scaffold
svonworl Mar 22, 2023
0527038
refactor into MarkdownWrapperService, add proof of concept code
svonworl Mar 22, 2023
faa38c5
develop further
svonworl Mar 23, 2023
456ab9e
develop
svonworl Mar 23, 2023
3f9f4dc
refactor lightly
svonworl Mar 23, 2023
b01d978
generalize formatting by type
svonworl Mar 23, 2023
dcf22f6
insert TODOs for further work, clean up
svonworl Mar 23, 2023
3298777
add mime bundle, baseUrl input
svonworl Mar 23, 2023
d2a9289
remove unused file
svonworl Mar 23, 2023
9530cb0
remove unused file #2
svonworl Mar 23, 2023
758eb83
add baseUrl input setting, adjust test
svonworl Mar 23, 2023
fdde446
style notebook divs, add exception handling on failure
svonworl Mar 24, 2023
121549f
make notebook css more scss-ey
svonworl Mar 24, 2023
fd6e4f2
correct execution count, lighten count label
svonworl Mar 24, 2023
d9bf2dd
add support for img alt and title attrs
svonworl Mar 24, 2023
2c5001b
refactor mime bundle processor
svonworl Mar 24, 2023
ce2c56b
rename method
svonworl Mar 24, 2023
9e94ed5
add math support
svonworl Mar 25, 2023
e5617bc
add syntax highlighting
svonworl Mar 25, 2023
c96564f
set language type of code cells, misc improvements
svonworl Mar 27, 2023
7f3c173
refactor escaping code
svonworl Mar 27, 2023
eb5113f
refactor
svonworl Mar 27, 2023
9f57869
honor source/outputs_hidden, add syntax highlighting styles, refactor
svonworl Mar 27, 2023
fdce41c
simplify, fix css problem
svonworl Mar 27, 2023
227980b
add test skeleton
svonworl Mar 28, 2023
4e9ea22
improve template html
svonworl Mar 28, 2023
133cc4c
restructure loop
svonworl Mar 28, 2023
0188552
complete basic unit tests, fix bugs
svonworl Mar 28, 2023
e7c8bc2
fix intermittent syntax highlighting
svonworl Mar 28, 2023
a389da8
fix misc bugs
svonworl Mar 29, 2023
a7bdc2a
improve escape
svonworl Mar 29, 2023
78e063b
refactor
svonworl Mar 29, 2023
d669962
add return type
svonworl Mar 29, 2023
e082dc3
refactor lightly
svonworl Mar 29, 2023
c034925
generate formatted notebook in detached element, add light sanitize
svonworl Mar 29, 2023
559ed62
fix license file, comment, tweak syntax highlighting
svonworl Mar 29, 2023
f169ee7
tweak license file
svonworl Mar 29, 2023
46c23d3
fix license file
svonworl Mar 29, 2023
5d8835e
fix bugs, make first grid column collapse to fit content
svonworl Mar 30, 2023
3f5f638
fix missing error
svonworl Mar 30, 2023
6e1c4cb
cancel old request subscription before launching new request
svonworl Mar 30, 2023
f8f4fda
rejigger attached image support
svonworl Mar 30, 2023
bb3f2dd
tweak styles, do not encode html output
svonworl Mar 30, 2023
2610a19
basic mathjax configurations
svonworl Mar 30, 2023
aef9135
escape trailing double backslashes in equations
svonworl Mar 31, 2023
90b6e3c
add mathjax font files, remove light sanitize
svonworl Mar 31, 2023
283b04d
add mathjax config file, configure ui/safe module, remove katex load
svonworl Apr 3, 2023
c5f6805
remove katex from package json
svonworl Apr 3, 2023
36b22a2
add spam removal
svonworl Apr 3, 2023
38eedba
disable mathjax automatic formatting
svonworl Apr 3, 2023
b43e9ac
use WorkflowService, comment, remove cruft
svonworl Apr 3, 2023
7fe9b3f
update license file
svonworl Apr 3, 2023
ca95810
add mathjax license, correctly this time
svonworl Apr 3, 2023
d951a0b
fix unit tests, address code robot suggestions
svonworl Apr 3, 2023
76b3f95
Merge branch 'develop' into feature/seab-5117/display-formatted-notebook
svonworl Apr 3, 2023
0d69931
remove commented code
svonworl Apr 3, 2023
d4d5577
remove unused stuff
svonworl Apr 4, 2023
c27419e
improve backslashed dollar handling
svonworl Apr 4, 2023
66e292d
improve regexps
svonworl Apr 4, 2023
96afbcb
improve regexps, add light sanitize of prism outputs
svonworl Apr 4, 2023
67e967d
refactor unit test
svonworl Apr 4, 2023
0212b63
clean up unit test
svonworl Apr 4, 2023
4ce9395
add comments, refactor lightly, fix regexp bug
svonworl Apr 4, 2023
d042404
fix spelling error
svonworl Apr 4, 2023
44389b3
change notebook Code tab to Preview tab
svonworl Apr 4, 2023
05a5cde
fix test oopsie
svonworl Apr 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
generate formatted notebook in detached element, add light sanitize
  • Loading branch information
svonworl committed Mar 29, 2023
commit c034925a7f4b241b0f5126b37f82ccb558774db9
58 changes: 40 additions & 18 deletions src/app/notebook/formatted-notebook.component.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { Component, ElementRef, Input, OnChanges, ViewChild } from '@angular/core';
import { FileService } from 'app/shared/file.service';
import { SourceFile, Workflow, WorkflowVersion } from 'app/shared/openapi';
import { Component, ElementRef, Inject, Input, OnChanges, ViewChild } from '@angular/core';
import { DOCUMENT } from '@angular/common';
import { finalize } from 'rxjs/operators';
import { Observable } from 'rxjs';
import { SourceFile, Workflow, WorkflowVersion } from 'app/shared/openapi';
import { SourceFileTabsService } from '../source-file-tabs/source-file-tabs.service';
import { WorkflowQuery } from '../shared/state/workflow.query';
import { Observable } from 'rxjs';
import { MarkdownWrapperService } from '../shared/markdown-wrapper/markdown-wrapper.service';
import { Renderer } from 'marked';
import DOMPurify from 'dompurify';

@Component({
selector: 'app-formatted-notebook',
Expand All @@ -15,25 +16,24 @@ import { Renderer } from 'marked';
})
export class FormattedNotebookComponent implements OnChanges {
constructor(
private fileService: FileService,
private sourceFileTabsService: SourceFileTabsService,
private markdownWrapperService: MarkdownWrapperService
private markdownWrapperService: MarkdownWrapperService,
@Inject(DOCUMENT) private document: Document
) {}
@Input() workflow: Workflow;
@Input() version: WorkflowVersion;
@Input() baseUrl: string;
@ViewChild('notebookTarget') notebookRef: ElementRef;
@ViewChild('notebookTarget') notebookTarget: ElementRef;
loading = true;
formatted = '';
displayError = false;

ngOnChanges() {
this.retrieveAndFormatNotebook();
}

retrieveAndFormatNotebook() {
this.notebookTarget?.nativeElement.replaceChildren();
this.loading = true;
this.formatted = '';
this.displayError = false;
this.sourceFileTabsService
.getSourceFiles(this.workflow.id, this.version.id)
Expand All @@ -45,16 +45,13 @@ export class FormattedNotebookComponent implements OnChanges {
.subscribe(
(sourceFiles: SourceFile[]) => {
for (const sourceFile of sourceFiles) {
// Look for the primary descriptor with Jupyter file type.
if (this.isPrimaryDescriptor(sourceFile.path) && sourceFile.type === SourceFile.TypeEnum.DOCKSTOREJUPYTER) {
try {
const notebookElement = this.notebookRef.nativeElement;
notebookElement.innerHTML = this.format(sourceFile.content);
for (const element of notebookElement.getElementsByClassName('markdown')) {
this.markdownWrapperService.katex(element);
}
for (const element of notebookElement.querySelectorAll('.source code')) {
this.markdownWrapperService.highlight(element);
}
// Create an element containing the formatted notebook,
// and make it the child of template's '#notebookTarget' placeholder.
const notebookElement = this.createFormattedNotebookElement(sourceFile.content);
this.notebookTarget.nativeElement.replaceChildren(notebookElement);
} catch (e) {
this.displayError = true;
console.log('Exception formatting notebook');
Expand All @@ -75,10 +72,31 @@ export class FormattedNotebookComponent implements OnChanges {
return path === this.version.workflow_path;
}

createFormattedNotebookElement(notebookJson: string): any {
// Create the notebook container div.
const element: any = this.document.createElement('div');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Couldn't you have something like this in the HTML template:

<div class="notebook" *ngIf="<condition>" [innerHtml]="sanitizedHtml">
<!--More stuff...-->
</div>

I find this more readable than tracing through TypeScript code to see what HTML will be generated.

Also I think it would be preferable to not have the the [innerHtml] here as in my example, but flesh out the template more -- I just used this as an example since it's the first case I ran into.

Copy link
Contributor Author

@svonworl svonworl Apr 5, 2023

Choose a reason for hiding this comment

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

Couldn't you have something like this in the HTML template:

<div class="notebook" *ngIf="<condition>" [innerHtml]="sanitizedHtml">
<!--More stuff...-->
</div>

I find this more readable than tracing through TypeScript code to see what HTML will be generated.

Also I think it would be preferable to not have the the [innerHtml] here as in my example, but flesh out the template more -- I just used this as an example since it's the first case I ran into.

Would "More stuff" in the above example be the "Couldn't display the template" message? Or are you proposing that the HTML template itself should contain at least some of the logic that steps through the notebook cells and converts them to output?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would "More stuff" in the above example be the "Couldn't display the template" message? Or are you proposing that the HTML template itself should contain at least some of the logic that steps through the notebook cells and converts them to output?

I'm proposing that the HTML template itself contain more of the logic and HTML.

element.classList.add('notebook');
// Set the body of the container div to the formatted notebook HTML.
element.innerHTML = this.format(notebookJson);
// Render the equations in each markdown cell.
for (const markdownElement of element.getElementsByClassName('markdown')) {
this.markdownWrapperService.katex(markdownElement);
}
// Highlight the code elements in the source of each code cell.
for (const codeElement of element.querySelectorAll('.source code')) {
this.markdownWrapperService.highlight(codeElement);
}
// Lightly sanitize the entirety of the inner HTML, conserving 'class' attribute values to
// prevent mangling the output of the equation rendering and syntax highlighting steps,
// which set the 'class' attribute on the elements they generate, so that they may be styled.
element.innerHTML = this.sanitizeLightly(element.innerHTML);
return element;
}

format(notebook: string): string {
const json = JSON.parse(notebook);
const divs = this.formatCells(json.cells);
return ['<div class="notebook">', ...divs, '</div>'].join('\n');
return divs.join('\n');
}

formatByType(values: any, typeField: string, typeToFormatter: Map<string, (json: any) => string[]>): string[] {
Expand Down Expand Up @@ -229,6 +247,10 @@ export class FormattedNotebookComponent implements OnChanges {
return this.markdownWrapperService.customSanitize(html);
}

sanitizeLightly(html: string): string {
return DOMPurify.sanitize(html, { FORBID_TAGS: [], FORBID_ATTR: [] });
}

// The below escape() implementation is adapted from mustache.js
// https://github.com/janl/mustache.js/blob/972fd2b27a036888acfcb60d6119317744fac7ee/mustache.js#L60
charToEntity = {
Expand Down
12 changes: 5 additions & 7 deletions src/app/shared/markdown-wrapper/markdown-wrapper.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,7 @@ export class MarkdownWrapperService {
forbidTags: string[] = [];
forbidAttr: string[] = ['class'];

constructor(private markdownService: MarkdownService, @Inject(DOCUMENT) private document: Document) {
DOMPurify.setConfig({
FORBID_TAGS: this.forbidTags,
FORBID_ATTR: this.forbidAttr,
});
}
constructor(private markdownService: MarkdownService, @Inject(DOCUMENT) private document: Document) {}

/**
* Compiles markdown into HTML with custom options.
Expand All @@ -43,7 +38,10 @@ export class MarkdownWrapperService {

customSanitize(html): string {
// Passes HTML to DOMPurify to be sanitized.
return DOMPurify.sanitize(html);
return DOMPurify.sanitize(html, {
FORBID_TAGS: this.forbidTags,
FORBID_ATTR: this.forbidAttr,
});
}

katex(element) {
Expand Down