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
Desktop: Resolves #7329: Fixes import of tasklists from enex files #7344
Conversation
packages/lib/import-enex-md-gen.ts
Outdated
@@ -35,8 +35,10 @@ interface ParserStateTag { | |||
isHighlight: boolean; | |||
} | |||
|
|||
type ListTag = 'ul' | 'ol' | 'checkboxList'; |
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.
Please make it an enum
packages/lib/import-enex-md-gen.ts
Outdated
@@ -738,7 +740,8 @@ function enexXmlToMdArray(stream: any, resources: ResourceEntity[]): Promise<Ene | |||
section.lines.push(BLOCK_OPEN); | |||
} else if (isListTag(n)) { | |||
section.lines.push(BLOCK_OPEN); | |||
state.lists.push({ tag: n, counter: 1, startedText: false }); | |||
const tag = nodeAttributes?.style?.toLowerCase().includes('--en-todo:true') ? 'checkboxList' : n as ListTag; |
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.
There's a cssValue function - could you use this please?
packages/lib/import-enex-md-gen.ts
Outdated
if (container.tag === 'ul') { | ||
|
||
if (container.tag === 'checkboxList') { | ||
const x = nodeAttributes?.style?.toLowerCase().includes('--en-checked:true') ? 'X' : ' '; |
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.
Use cssValue()
packages/lib/import-enex-md-gen.ts
Outdated
@@ -738,7 +744,9 @@ function enexXmlToMdArray(stream: any, resources: ResourceEntity[]): Promise<Ene | |||
section.lines.push(BLOCK_OPEN); | |||
} else if (isListTag(n)) { | |||
section.lines.push(BLOCK_OPEN); | |||
state.lists.push({ tag: n, counter: 1, startedText: false }); | |||
const isCheckboxList = cssValue(this, nodeAttributes?.style, '--en-todo') === 'true'; |
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.
nodeAttributes is always defined (or should be) so please remove the "?"
packages/lib/import-enex-md-gen.ts
Outdated
if (container.tag === 'ul') { | ||
|
||
if (container.tag === ListTag.CheckboxList) { | ||
const x = cssValue(this, nodeAttributes?.style, '--en-checked') === 'true' ? 'X' : ' '; |
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.
Remove the "?"
packages/lib/import-enex-html-gen.js
Outdated
const checkedHtml = nodeAttributes.checked && nodeAttributes.checked.toLowerCase() === 'true' ? ' checked="checked" ' : ' '; | ||
section.lines.push(`<input${checkedHtml}type="checkbox" onclick="return false;" />`); | ||
} else if (tagName === 'li' && nodeAttributes?.style?.includes('--en-checked:')) { |
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.
Remove the "?s". We need to know what data we're working with, not just assume everything's broken all the time. If something's undefined we need to know why and fix it, not just ignore it with "?".
packages/lib/import-enex-html-gen.js
Outdated
const checkedHtml = nodeAttributes.checked && nodeAttributes.checked.toLowerCase() === 'true' ? ' checked="checked" ' : ' '; | ||
section.lines.push(`<input${checkedHtml}type="checkbox" onclick="return false;" />`); | ||
} else if (tagName === 'li' && nodeAttributes?.style?.includes('--en-checked:')) { | ||
const checkedHtml = nodeAttributes.style.includes('--en-checked:true') ? ' checked="checked" ' : ' '; |
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 think you can use cssValue() here as well
Looks good now, thanks a lot for the fix! |
Checkboxes, which Joplin already handles correctly, and checklists. which were added more recently to Evernote. A checklists is a
ul
, with a style that indicates it's a list of checkboxes. Right now Joplin imports them as normalul
s, but since Evernote treats them like checkboxlists, it makes sense for Joplin to do the same.Some more info and a small discussion can be found int the issue #7329
It seems pretty easy to change import-enex-html-gen.js to a TS file, but I didn't do that because it looks like import-enex-md-gen could use a few changes as well, so I thought I'd do that in a separate pr.