Conversation
|
Great stuff! I'll review https://github.com/solid/solid-ui/pull/206 first and then this one. |
package.json
Outdated
| "hooks": { | ||
| "pre-commit": "lint-staged", | ||
| "pre-push": "npm test" | ||
| "pre-push-disabled": "npm test" |
There was a problem hiding this comment.
Please exclude this change from your PR
| "folder-pane": "^2.0.1", | ||
| "issue-pane": "^2.0.0", | ||
| "meeting-pane": "^2.0.0", | ||
| "meeting-pane": "^2.0.7", |
There was a problem hiding this comment.
Please exclude this change from your PR
| @@ -1539,9 +1539,9 @@ | |||
| } | |||
| }, | |||
| "@types/babel__core": { | |||
There was a problem hiding this comment.
Please exclude this change from your PR
src/internal/internalPane.ts
Outdated
| // import { IndexedFormula, literal, NamedNode } from 'rdflib' // import/no-duplicates | ||
| import * as $rdf from 'rdflib' | ||
| import { PaneDefinition } from 'pane-registry' | ||
| const { IndexedFormula, NamedNode, literal } = $rdf |
There was a problem hiding this comment.
What is internalPane, how is it related to the pad pane, and why do you propose we add the $rdf global to this file, rather than referring to the individual imports like IndexedFormula, NamedNode, etc directly, like it was?
src/internal/internalPane.ts
Outdated
| $rdf.st( | ||
| subject, | ||
| sym('http://www.w3.org/2007/ont/link#uri'), | ||
| store.sym('http://www.w3.org/2007/ont/link#uri'), |
There was a problem hiding this comment.
Does store.sym do the same as $rdf.namedNode or is there a difference?
| @@ -1,441 +0,0 @@ | |||
| /** | |||
There was a problem hiding this comment.
Was this code unused? If so, then yes please delete it! But we should make it a separate PR, since this is unrelated to the pad pane.
src/outline/manager.js
Outdated
| Outline Mode Manager | ||
| */ | ||
| var panes = require('pane-registry') | ||
| const panes = require('pane-registry') |
There was a problem hiding this comment.
These changes seem unrelated as well?
We can do this kind of cleanup at the same time as moving this file to typescript, adding unit tests for it, fixing up its documentation, and adding examples for how to use the functions it exports.
src/pad/padPane.ts
Outdated
| @@ -1,8 +1,10 @@ | |||
| import { authn, icons, ns, pad, widgets } from 'solid-ui' | |||
| import { authn, icons, ns, participation, pad, utils, widgets } from 'solid-ui' | |||
There was a problem hiding this comment.
These changes will not be necessary if we exclude the refactor work from https://github.com/solid/solid-ui/pull/206, like I did in https://github.com/solid/solid-ui/pull/211.
|
|
||
| /** Button to Export the pad to HTML | ||
| */ | ||
| var exportHTMLButton = function (subject) { |
There was a problem hiding this comment.
Great! I'll test this.
src/pad/padPane.ts
Outdated
| const copyIcon = icons.iconBase + 'noun_681601.svg' | ||
| return widgets.button(dom, copyIcon, 'Copy as HTML', | ||
| async function (_event) { | ||
| const htmlText = pad.notePadToHTML(subject, store) |
There was a problem hiding this comment.
Might need a lower-case p if we merge solid-contrib/solid-ui@a79b324
michielbdejong
left a comment
michielbdejong
left a comment
There was a problem hiding this comment.
After we merge https://github.com/solid/solid-ui/pull/211 I'll create a new PR based on this one, that only contains the changes for adding the 'export to HTML' button, and then we can split the unrelated refactor work into more rigorous cleanup PRs.
|
Now should be just 1 file changed |
|
Travis reports errors: ERROR in /home/travis/build/solid/solid-panes/src/pad/padPane.ts |
|
https://github.com/solid/solid-panes/pull/225/files still shows 4 files changed |
3 participants
Add the ability to make an HTML export of the notepad.