mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-01-23 02:36:05 +00:00
fix(ui): make checklist items clickable after blank lines
- Fix clickPreview() to use closest('.checkbox-wrapper') so clicking
anywhere on a checklist item (text or icon) toggles the checkbox
- Add cursor: pointer to .checkbox-wrapper for better UX
- Add comprehensive tests for checkbox click handling (21 tests)
- Add tests for marked-options-factory checkbox rendering (11 tests)
Fixes #5950
This commit is contained in:
parent
b69dded9f1
commit
c8fec79448
4 changed files with 391 additions and 10 deletions
|
|
@ -275,7 +275,7 @@ describe('InlineMarkdownComponent', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('clickPreview with loose lists (blank lines)', () => {
|
||||
describe('clickPreview', () => {
|
||||
let mockPreviewEl: { element: { nativeElement: HTMLElement } };
|
||||
|
||||
beforeEach(() => {
|
||||
|
|
@ -326,5 +326,259 @@ describe('InlineMarkdownComponent', () => {
|
|||
// Assert - Task 2 should be toggled
|
||||
expect(component.changed.emit).toHaveBeenCalledWith('- [ ] Task 1\n\n- [x] Task 2');
|
||||
});
|
||||
|
||||
it('should toggle checkbox when clicking on the label text (not just the checkbox icon)', () => {
|
||||
// Arrange
|
||||
component.model = '- [ ] Task 1\n- [ ] Task 2';
|
||||
fixture.detectChanges();
|
||||
|
||||
// Build DOM structure
|
||||
const wrapper1 = document.createElement('li');
|
||||
wrapper1.className = 'checkbox-wrapper undone';
|
||||
const checkbox1 = document.createElement('span');
|
||||
checkbox1.className = 'checkbox material-icons';
|
||||
checkbox1.textContent = 'check_box_outline_blank';
|
||||
const textNode1 = document.createTextNode('Task 1');
|
||||
wrapper1.appendChild(checkbox1);
|
||||
wrapper1.appendChild(textNode1);
|
||||
|
||||
const wrapper2 = document.createElement('li');
|
||||
wrapper2.className = 'checkbox-wrapper undone';
|
||||
const checkbox2 = document.createElement('span');
|
||||
checkbox2.className = 'checkbox material-icons';
|
||||
checkbox2.textContent = 'check_box_outline_blank';
|
||||
const textSpan2 = document.createElement('span');
|
||||
textSpan2.textContent = 'Task 2';
|
||||
wrapper2.appendChild(checkbox2);
|
||||
wrapper2.appendChild(textSpan2);
|
||||
|
||||
mockPreviewEl.element.nativeElement.appendChild(wrapper1);
|
||||
mockPreviewEl.element.nativeElement.appendChild(wrapper2);
|
||||
|
||||
// Act - simulate clicking on the text span (not the checkbox icon)
|
||||
const mockEvent = {
|
||||
target: textSpan2,
|
||||
} as unknown as MouseEvent;
|
||||
component.clickPreview(mockEvent);
|
||||
|
||||
// Assert - Task 2 should be toggled
|
||||
expect(component.changed.emit).toHaveBeenCalledWith('- [ ] Task 1\n- [x] Task 2');
|
||||
});
|
||||
|
||||
it('should toggle checkbox when clicking directly on the checkbox-wrapper element', () => {
|
||||
// Arrange
|
||||
component.model = '- [ ] Task 1';
|
||||
fixture.detectChanges();
|
||||
|
||||
const wrapper1 = document.createElement('li');
|
||||
wrapper1.className = 'checkbox-wrapper undone';
|
||||
const checkbox1 = document.createElement('span');
|
||||
checkbox1.className = 'checkbox material-icons';
|
||||
checkbox1.textContent = 'check_box_outline_blank';
|
||||
wrapper1.appendChild(checkbox1);
|
||||
wrapper1.appendChild(document.createTextNode('Task 1'));
|
||||
|
||||
mockPreviewEl.element.nativeElement.appendChild(wrapper1);
|
||||
|
||||
// Act - simulate clicking directly on the wrapper
|
||||
const mockEvent = {
|
||||
target: wrapper1,
|
||||
} as unknown as MouseEvent;
|
||||
component.clickPreview(mockEvent);
|
||||
|
||||
// Assert
|
||||
expect(component.changed.emit).toHaveBeenCalledWith('- [x] Task 1');
|
||||
});
|
||||
|
||||
it('should not toggle checkbox when clicking on a link', () => {
|
||||
// Arrange
|
||||
component.model = '- [ ] Task with [link](http://example.com)';
|
||||
fixture.detectChanges();
|
||||
|
||||
const wrapper1 = document.createElement('li');
|
||||
wrapper1.className = 'checkbox-wrapper undone';
|
||||
const checkbox1 = document.createElement('span');
|
||||
checkbox1.className = 'checkbox material-icons';
|
||||
checkbox1.textContent = 'check_box_outline_blank';
|
||||
const link = document.createElement('a');
|
||||
link.href = 'http://example.com';
|
||||
link.textContent = 'link';
|
||||
wrapper1.appendChild(checkbox1);
|
||||
wrapper1.appendChild(document.createTextNode('Task with '));
|
||||
wrapper1.appendChild(link);
|
||||
|
||||
mockPreviewEl.element.nativeElement.appendChild(wrapper1);
|
||||
|
||||
// Act - simulate clicking on the link
|
||||
const mockEvent = {
|
||||
target: link,
|
||||
} as unknown as MouseEvent;
|
||||
component.clickPreview(mockEvent);
|
||||
|
||||
// Assert - checkbox should NOT be toggled (link should work normally)
|
||||
expect(component.changed.emit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should toggle edit mode when clicking outside checkbox-wrapper', () => {
|
||||
// Arrange
|
||||
component.model = 'Some regular text';
|
||||
fixture.detectChanges();
|
||||
|
||||
const paragraph = document.createElement('p');
|
||||
paragraph.textContent = 'Some regular text';
|
||||
mockPreviewEl.element.nativeElement.appendChild(paragraph);
|
||||
|
||||
spyOn<any>(component, '_toggleShowEdit');
|
||||
|
||||
// Act - simulate clicking on regular text
|
||||
const mockEvent = {
|
||||
target: paragraph,
|
||||
} as unknown as MouseEvent;
|
||||
component.clickPreview(mockEvent);
|
||||
|
||||
// Assert
|
||||
expect(component['_toggleShowEdit']).toHaveBeenCalled();
|
||||
expect(component.changed.emit).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('_handleCheckboxClick edge cases', () => {
|
||||
let mockPreviewEl: { element: { nativeElement: HTMLElement } };
|
||||
|
||||
beforeEach(() => {
|
||||
mockPreviewEl = {
|
||||
element: {
|
||||
nativeElement: document.createElement('div'),
|
||||
},
|
||||
};
|
||||
spyOn(component, 'previewEl').and.returnValue(mockPreviewEl as any);
|
||||
spyOn(component.changed, 'emit');
|
||||
});
|
||||
|
||||
it('should preserve blank lines when toggling checkboxes', () => {
|
||||
// Arrange
|
||||
component.model = '- [ ] Task 1\n\n- [ ] Task 2\n\n- [ ] Task 3';
|
||||
fixture.detectChanges();
|
||||
|
||||
const wrapper1 = document.createElement('li');
|
||||
wrapper1.className = 'checkbox-wrapper';
|
||||
const wrapper2 = document.createElement('li');
|
||||
wrapper2.className = 'checkbox-wrapper';
|
||||
const wrapper3 = document.createElement('li');
|
||||
wrapper3.className = 'checkbox-wrapper';
|
||||
|
||||
mockPreviewEl.element.nativeElement.appendChild(wrapper1);
|
||||
mockPreviewEl.element.nativeElement.appendChild(wrapper2);
|
||||
mockPreviewEl.element.nativeElement.appendChild(wrapper3);
|
||||
|
||||
// Act - toggle Task 2
|
||||
component['_handleCheckboxClick'](wrapper2);
|
||||
|
||||
// Assert - blank lines should be preserved
|
||||
expect(component.changed.emit).toHaveBeenCalledWith(
|
||||
'- [ ] Task 1\n\n- [x] Task 2\n\n- [ ] Task 3',
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle mixed checked and unchecked items', () => {
|
||||
// Arrange
|
||||
component.model = '- [x] Done\n- [ ] Todo\n- [x] Also Done';
|
||||
fixture.detectChanges();
|
||||
|
||||
const wrapper1 = document.createElement('li');
|
||||
wrapper1.className = 'checkbox-wrapper';
|
||||
const wrapper2 = document.createElement('li');
|
||||
wrapper2.className = 'checkbox-wrapper';
|
||||
const wrapper3 = document.createElement('li');
|
||||
wrapper3.className = 'checkbox-wrapper';
|
||||
|
||||
mockPreviewEl.element.nativeElement.appendChild(wrapper1);
|
||||
mockPreviewEl.element.nativeElement.appendChild(wrapper2);
|
||||
mockPreviewEl.element.nativeElement.appendChild(wrapper3);
|
||||
|
||||
// Act - toggle the middle item (Todo -> Done)
|
||||
component['_handleCheckboxClick'](wrapper2);
|
||||
|
||||
// Assert
|
||||
expect(component.changed.emit).toHaveBeenCalledWith(
|
||||
'- [x] Done\n- [x] Todo\n- [x] Also Done',
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle checklist with text before it', () => {
|
||||
// Arrange
|
||||
component.model = 'Some intro text\n\n- [ ] Task 1\n- [ ] Task 2';
|
||||
fixture.detectChanges();
|
||||
|
||||
const wrapper1 = document.createElement('li');
|
||||
wrapper1.className = 'checkbox-wrapper';
|
||||
const wrapper2 = document.createElement('li');
|
||||
wrapper2.className = 'checkbox-wrapper';
|
||||
|
||||
mockPreviewEl.element.nativeElement.appendChild(wrapper1);
|
||||
mockPreviewEl.element.nativeElement.appendChild(wrapper2);
|
||||
|
||||
// Act
|
||||
component['_handleCheckboxClick'](wrapper1);
|
||||
|
||||
// Assert
|
||||
expect(component.changed.emit).toHaveBeenCalledWith(
|
||||
'Some intro text\n\n- [x] Task 1\n- [ ] Task 2',
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle checklist with text after it', () => {
|
||||
// Arrange
|
||||
component.model = '- [ ] Task 1\n- [ ] Task 2\n\nSome outro text';
|
||||
fixture.detectChanges();
|
||||
|
||||
const wrapper1 = document.createElement('li');
|
||||
wrapper1.className = 'checkbox-wrapper';
|
||||
const wrapper2 = document.createElement('li');
|
||||
wrapper2.className = 'checkbox-wrapper';
|
||||
|
||||
mockPreviewEl.element.nativeElement.appendChild(wrapper1);
|
||||
mockPreviewEl.element.nativeElement.appendChild(wrapper2);
|
||||
|
||||
// Act
|
||||
component['_handleCheckboxClick'](wrapper2);
|
||||
|
||||
// Assert
|
||||
expect(component.changed.emit).toHaveBeenCalledWith(
|
||||
'- [ ] Task 1\n- [x] Task 2\n\nSome outro text',
|
||||
);
|
||||
});
|
||||
|
||||
it('should not emit if model is undefined', () => {
|
||||
// Arrange
|
||||
component.model = undefined;
|
||||
fixture.detectChanges();
|
||||
|
||||
const wrapper1 = document.createElement('li');
|
||||
wrapper1.className = 'checkbox-wrapper';
|
||||
mockPreviewEl.element.nativeElement.appendChild(wrapper1);
|
||||
|
||||
// Act
|
||||
component['_handleCheckboxClick'](wrapper1);
|
||||
|
||||
// Assert
|
||||
expect(component.changed.emit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not emit if clicked element is not found in DOM', () => {
|
||||
// Arrange
|
||||
component.model = '- [ ] Task 1';
|
||||
fixture.detectChanges();
|
||||
|
||||
// Create a wrapper that's NOT in the previewEl
|
||||
const orphanWrapper = document.createElement('li');
|
||||
orphanWrapper.className = 'checkbox-wrapper';
|
||||
|
||||
// Act
|
||||
component['_handleCheckboxClick'](orphanWrapper);
|
||||
|
||||
// Assert
|
||||
expect(component.changed.emit).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -150,15 +150,16 @@ export class InlineMarkdownComponent implements OnInit, OnDestroy {
|
|||
|
||||
clickPreview($event: MouseEvent): void {
|
||||
if (($event.target as HTMLElement).tagName === 'A') {
|
||||
// } else if (($event.target as HTMLElement).classList.contains('checkbox-wrapper')) {
|
||||
// this._handleCheckboxClick($event.target as HTMLElement);
|
||||
} else if (
|
||||
$event?.target &&
|
||||
($event.target as HTMLElement).classList.contains('checkbox')
|
||||
) {
|
||||
this._handleCheckboxClick(
|
||||
($event.target as HTMLElement).parentElement as HTMLElement,
|
||||
);
|
||||
// Let links work normally
|
||||
return;
|
||||
}
|
||||
|
||||
// Check if click is anywhere inside a checkbox-wrapper (text or checkbox icon)
|
||||
const wrapper = ($event.target as HTMLElement).closest(
|
||||
'.checkbox-wrapper',
|
||||
) as HTMLElement;
|
||||
if (wrapper) {
|
||||
this._handleCheckboxClick(wrapper);
|
||||
} else {
|
||||
this._toggleShowEdit();
|
||||
}
|
||||
|
|
|
|||
125
src/app/ui/marked-options-factory.spec.ts
Normal file
125
src/app/ui/marked-options-factory.spec.ts
Normal file
|
|
@ -0,0 +1,125 @@
|
|||
import { markedOptionsFactory } from './marked-options-factory';
|
||||
|
||||
describe('markedOptionsFactory', () => {
|
||||
let options: ReturnType<typeof markedOptionsFactory>;
|
||||
|
||||
beforeEach(() => {
|
||||
options = markedOptionsFactory();
|
||||
});
|
||||
|
||||
it('should return a MarkedOptions object', () => {
|
||||
expect(options).toBeDefined();
|
||||
expect(options.renderer).toBeDefined();
|
||||
expect(options.gfm).toBe(true);
|
||||
});
|
||||
|
||||
describe('checkbox renderer', () => {
|
||||
it('should render unchecked checkbox with material icon', () => {
|
||||
const result = options.renderer!.checkbox({ checked: false } as any);
|
||||
expect(result).toContain('check_box_outline_blank');
|
||||
expect(result).toContain('class="checkbox material-icons"');
|
||||
});
|
||||
|
||||
it('should render checked checkbox with material icon', () => {
|
||||
const result = options.renderer!.checkbox({ checked: true } as any);
|
||||
expect(result).toContain('check_box');
|
||||
expect(result).not.toContain('check_box_outline_blank');
|
||||
expect(result).toContain('class="checkbox material-icons"');
|
||||
});
|
||||
});
|
||||
|
||||
describe('listitem renderer', () => {
|
||||
it('should render regular list item without checkbox', () => {
|
||||
const result = options.renderer!.listitem({
|
||||
text: 'Regular item',
|
||||
task: false,
|
||||
checked: undefined,
|
||||
} as any);
|
||||
expect(result).toBe('<li>Regular item</li>');
|
||||
expect(result).not.toContain('checkbox-wrapper');
|
||||
});
|
||||
|
||||
it('should render unchecked task list item with checkbox', () => {
|
||||
const result = options.renderer!.listitem({
|
||||
text: 'Task item',
|
||||
task: true,
|
||||
checked: false,
|
||||
} as any);
|
||||
expect(result).toContain('checkbox-wrapper');
|
||||
expect(result).toContain('undone');
|
||||
expect(result).toContain('check_box_outline_blank');
|
||||
expect(result).toContain('Task item');
|
||||
});
|
||||
|
||||
it('should render checked task list item with checkbox', () => {
|
||||
const result = options.renderer!.listitem({
|
||||
text: 'Completed task',
|
||||
task: true,
|
||||
checked: true,
|
||||
} as any);
|
||||
expect(result).toContain('checkbox-wrapper');
|
||||
expect(result).toContain('done');
|
||||
expect(result).not.toContain('undone');
|
||||
expect(result).toContain('check_box');
|
||||
expect(result).not.toContain('check_box_outline_blank');
|
||||
expect(result).toContain('Completed task');
|
||||
});
|
||||
|
||||
it('should handle undefined checked value as unchecked', () => {
|
||||
const result = options.renderer!.listitem({
|
||||
text: 'Task with undefined checked',
|
||||
task: true,
|
||||
checked: undefined,
|
||||
} as any);
|
||||
expect(result).toContain('checkbox-wrapper');
|
||||
expect(result).toContain('undone');
|
||||
expect(result).toContain('check_box_outline_blank');
|
||||
});
|
||||
});
|
||||
|
||||
describe('link renderer', () => {
|
||||
it('should render link with target="_blank"', () => {
|
||||
const result = options.renderer!.link({
|
||||
href: 'http://example.com',
|
||||
title: 'Example',
|
||||
text: 'Click here',
|
||||
} as any);
|
||||
expect(result).toContain('target="_blank"');
|
||||
expect(result).toContain('href="http://example.com"');
|
||||
expect(result).toContain('title="Example"');
|
||||
expect(result).toContain('Click here');
|
||||
});
|
||||
|
||||
it('should handle empty title', () => {
|
||||
const result = options.renderer!.link({
|
||||
href: 'http://example.com',
|
||||
title: null,
|
||||
text: 'Link',
|
||||
} as any);
|
||||
expect(result).toContain('title=""');
|
||||
});
|
||||
});
|
||||
|
||||
describe('text renderer with URL auto-linking', () => {
|
||||
it('should auto-link URLs in text', () => {
|
||||
const result = options.renderer!.text({
|
||||
text: 'Check out http://example.com for more info',
|
||||
type: 'text',
|
||||
raw: 'Check out http://example.com for more info',
|
||||
} as any);
|
||||
// The text renderer modifies the token and passes it to the original renderer
|
||||
// which may HTML-escape the content, so we check for the href pattern
|
||||
expect(result).toContain('http://example.com');
|
||||
expect(result).toContain('href=');
|
||||
});
|
||||
|
||||
it('should handle text without URLs', () => {
|
||||
const result = options.renderer!.text({
|
||||
text: 'Regular text without links',
|
||||
type: 'text',
|
||||
raw: 'Regular text without links',
|
||||
} as any);
|
||||
expect(result).not.toContain('href=');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -112,6 +112,7 @@
|
|||
display: block;
|
||||
list-style: none;
|
||||
padding-bottom: var(--s);
|
||||
cursor: pointer;
|
||||
|
||||
&.done {
|
||||
opacity: 0.6;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue