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

(Bug report) redundent new line on long check-box item #4402

Closed
Nriver opened this issue Nov 7, 2023 · 8 comments
Closed

(Bug report) redundent new line on long check-box item #4402

Nriver opened this issue Nov 7, 2023 · 8 comments

Comments

@Nriver
Copy link
Contributor

Nriver commented Nov 7, 2023

Trilium Version

0.61.13

What operating system are you using?

Windows

What is your setup?

Local + server sync

Operating System Version

Win 10

Description

If the todo item is longer than one line, the text is pushed to a new line.

ksnip_20231107-103758

Error logs

No response

@Nriver
Copy link
Contributor Author

Nriver commented Nov 7, 2023

I've checked the ckeditor5's online demo. That works fine. So I assume this is a trilium issue.

https://ckeditor.com/docs/ckeditor5/latest/features/lists/todo-lists.html
ksnip_20231107-104322

@mabeyj
Copy link

mabeyj commented Nov 7, 2023

I upgraded my main setup today from 0.60.4 to 0.61.13, and I'm also seeing similar weirdness with todo lists. It seems like the todo markup is getting mangled when a note is opened.

If I create a new todo list in a new note, it works fine and the HTML is fine:

If I switch to a different note and then back to the new note, now the todo list has extra label and label__description elements inserted into each item's description:

I have a copy of 0.61.6 for testing, and this issue doesn't occur in that version.

@Nriver
Copy link
Contributor Author

Nriver commented Nov 7, 2023

For a termporary fix, I added a css note with #appCss label.

The css content:

label.todo-list__label {
    display: initial;
}

ksnip_20231107-134659

@Nriver
Copy link
Contributor Author

Nriver commented Nov 8, 2023

0.60.4 The source code of todo item is like this.

<ul class="todo-list">
	<li>
		<label class="todo-list__label">
			<input type="checkbox" disabled="disabled" checked="checked">
			<span class="todo-list__label__description">
				0.60.4
			</span>
		</label>
	</li>

0.61.13 The current state of the todo item includes an unwanted cascaded todo-list__label. This issue can be reproduced by checking and unchecking the checkbox.

<ul class="todo-list">
	<li>
		<label class="todo-list__label">
			<input type="checkbox" disabled="disabled">
			<span class="todo-list__label__description">
				<label class="todo-list__label">
					<span class="todo-list__label__description">
						0.61.13
					</span>
				</label>
			</span>
		</label>
	</li>

@mabeyj
Copy link

mabeyj commented Nov 8, 2023

Wrote a widget that fixes the HTML when a note is opened. This loops over each todo item and removes the extra todo-list__label and todo-list__label__description elements that are being inserted.

class FixTodoListWidget extends api.NoteContextAwareWidget {
    get parentWidget() { return "left-pane"; }

    doRender() {
        this.$widget = $("");
        this.editorIntervalId = null;
        this.initializedEditorIds = [];
    }

    refreshWithNote() {
        if (this.note.type === "text") {
            this.initializeEditor();
        }
    }

    initializeEditor() {
        this.editorIntervalId = setInterval(async () => {
            const editor = await api.getActiveContextTextEditor();
            clearInterval(this.editorIntervalId);

            if (this.initializedEditorIds.includes(editor.id)) {
                return;
            }
            this.initializedEditorIds.push(editor.id);

            editor.data.on('set', () => this.fixTodoLists(editor));
            this.fixTodoLists(editor);
        }, 100);
    }

    fixTodoLists(editor) {
        let brokenElements = [];
        this.findBrokenElements(editor.model.document.getRoot(), brokenElements);
        if (!brokenElements.length) {
            return;
        }

        editor.model.change((writer) => {
            for (const element of brokenElements) {
                this.fixElement(writer, element);
            }
        });
    }
                                   
    findBrokenElements(node, brokenElements) {
        if (!node.is("element")) {
            return;
        }

        const isTodoItem = node.getAttribute("listType") === "todo";
        for (const child of node.getChildren()) {
            if (isTodoItem && (child.hasAttribute("htmlLabel") || child.hasAttribute("htmlSpan"))) {
                brokenElements.push(child);
            }
            this.findBrokenElements(child, brokenElements);
        }
    }

    fixElement(writer, element) {
        for (const key of ["htmlLabel", "htmlSpan"]) {
            if (element.hasAttribute(key)) {
                writer.removeAttribute(key, element);
            }
        }
    }
}

module.exports = new FixTodoListWidget();

@zadam
Copy link
Owner

zadam commented Nov 12, 2023

Hmm, Trilium itself does nothing with such HTML content and has no transformation like that.

0.61 contains CKEditor upgrade (from 36.4.0 to 40.0.0) which could be doing this. I have a suspicion this might be connected to the fact that CKEditor is currently undergoing some major lists refactoring. CKEditor in Trilium should still be using the old list/todo plugin, though ...

@Nriver
Copy link
Contributor Author

Nriver commented Nov 16, 2023

#4402 (comment)

Just for memo: set #widget #run=mobileStartup to make it work on both desktop client and mobile browser.

@Nriver
Copy link
Contributor Author

Nriver commented Nov 27, 2023

Closing this as it is fixed in the latest version.

@Nriver Nriver closed this as completed Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants