Skip to content

Advanced checklist fixes #50

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.

Sign up for GitHub

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

Merged
antsgar merged 5 commits into from
Sep 11, 2025
Merged

Advanced checklist fixes #50

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b5e4c06
fix: Fixes Advanced Checklist plugin not rendering on iOS and Safari
antsgar Aug 28, 2025
5ff0738
fix: Fixes Redux non-serializable value error when creating checklist…
antsgar Aug 28, 2025
6f3d8ff
fix: Fixed Advanced Checklist plugin not rendering on iOS and Safari
antsgar Sep 2, 2025
ee923c8
chore: fix tests and add tests for backwards compatibility
antsgar Sep 10, 2025
58a65ca
chore: remove logs
antsgar Sep 10, 2025
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
30 changes: 15 additions & 15 deletions packages/com.sncommunity.advanced-checklist/src/common/utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,18 +92,18 @@ describe('groupTasksByCompletedStatus', () => {
id: 'test-1',
description: 'Testing #1',
completed: false,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
{
id: 'test-2',
description: 'Testing #2',
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
{
id: 'test-3',
description: 'Testing #3',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
]

Expand All @@ -124,27 +124,27 @@ describe('getTaskArrayFromGroupedTasks', () => {
{
id: 'test-b-1',
description: 'Test #1',
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
{
id: 'test-b-2',
description: 'Test #2',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
]

const personalTasks = [
{
id: 'test-c-1',
description: 'Test #3',
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
{
id: 'test-c-2',
description: 'Test #4',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
]

Expand Down Expand Up @@ -190,32 +190,32 @@ describe('getPlainPreview', () => {
{
id: 'test-b-1',
description: 'Test #1',
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
{
id: 'test-b-2',
description: 'Test #2',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
]

const personalTasks = [
{
id: 'test-c-1',
description: 'Test #3',
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
{
id: 'test-c-2',
description: 'Test #4',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
{
id: 'test-c-3',
description: 'Test #5',
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
]

Expand Down Expand Up @@ -263,19 +263,19 @@ describe('parseMarkdownTasks', () => {
id: expect.any(String),
description: 'Foo',
completed: false,
createdAt: expect.any(Date),
createdAt: expect.any(String),
},
{
id: expect.any(String),
description: 'Bar',
completed: true,
createdAt: expect.any(Date),
createdAt: expect.any(String),
},
{
id: expect.any(String),
description: 'Foobar',
completed: false,
createdAt: expect.any(Date),
createdAt: expect.any(String),
},
],
sections: DEFAULT_SECTIONS,
Expand Down
4 changes: 2 additions & 2 deletions packages/com.sncommunity.advanced-checklist/src/common/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ function createTaskFromLine(rawTask: string): TaskModel | undefined {
id: uuidv4(),
description,
completed: IS_COMPLETED.test(rawTask),
createdAt: new Date(),
createdAt: new Date().toISOString(),
Copy link
Member

@amanharwara amanharwara Aug 28, 2025

Choose a reason for hiding this comment

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

How will this work with existing notes?

Copy link
Contributor Author

@antsgar antsgar Aug 29, 2025

Choose a reason for hiding this comment

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

Great question! The "non-serializable values" message from Redux is more of a warning than an error I think, as this was working until now and I only discovered the message by running the plugin locally. Existing notes will still have dates as createdAt and will trigger the warning, but the comparison in line 130 in this file should still work okay since doing new Date() on a date will do nothing. We could also "migrate" the createdAt values of existing notes to strings, but I'd say it's not worth it -- if I'm understanding correctly, the only thing that lastActive comparison does for now is trigger the autofocus on the input for the last group.

Copy link
Member

@amanharwara amanharwara Aug 29, 2025

Choose a reason for hiding this comment

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

Cool, sounds good. As long as we can be fairly certain that existing notes won't break its fine. Also, nice work figuring out what was causing this, it has been broken for a while. Was it the dispatch(tasksLoaded('{}')) that ended up fixing it?

Copy link
Contributor Author

@antsgar antsgar Sep 10, 2025

Choose a reason for hiding this comment

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

Added some tests to ensure the backwards compatibility of the Date to string change!

As for the actual fix for the editor not rendering problem, what I had proposed here originally was the wrong way to go and would have only fixed the initial render while causing problems for existing notes. Turns out the cause of the issue is a race condition where the iframe onLoad event was being called before the editor called configureEditorKit. This meant that the component was being registered before there was any listener setup to handle that event, and get the rest of the flow going. It's a mystery to me though why this is only happening on iOS/Safari - perhaps the iframe is loading faster, or the event happening at a different time for some reason? At any rate, I noticed that this problem isn't happening on other editors that also make use of the editor kit and configureEditorKit, such as Markdown Visual and Token Vault. The difference is those use class components, and were calling the function either in the constructor or in componentDidMount. Here we were using useEffect, which runs slightly later only after the initial paint. Changing it to useLayoutEffect ensures the editor kit is configured earlier, which fixes the issue.

}
}

Expand Down Expand Up @@ -127,7 +127,7 @@ export function isLastActiveGroup(allGroups: GroupModel[], groupName: string): b
if (!current.lastActive) {
return prev
}
return prev.lastActive > current.lastActive ? prev : current
return new Date(prev.lastActive) > new Date(current.lastActive) ? prev : current
})

return lastActiveGroup.name === groupName
Expand Down
20 changes: 10 additions & 10 deletions packages/com.sncommunity.advanced-checklist/src/features/tasks/MergeTaskGroups.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ it('renders the alert dialog when no groups are available to merge', () => {
id: 'some-id',
description: 'A simple task',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
},
Expand Down Expand Up @@ -54,7 +54,7 @@ it('renders the alert dialog when there are groups available to merge', () => {
id: 'some-id',
description: 'A simple task',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
},
Expand All @@ -65,7 +65,7 @@ it('renders the alert dialog when there are groups available to merge', () => {
id: 'another-id',
description: 'Another simple task',
completed: false,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
},
Expand All @@ -76,7 +76,7 @@ it('renders the alert dialog when there are groups available to merge', () => {
id: 'yet-another-id',
description: 'Yet another simple task',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
},
Expand Down Expand Up @@ -117,7 +117,7 @@ it('should close the dialog if no group is selected and the Merge button is clic
id: 'some-id',
description: 'A simple task',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
},
Expand All @@ -128,7 +128,7 @@ it('should close the dialog if no group is selected and the Merge button is clic
id: 'another-id',
description: 'Another simple task',
completed: false,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
},
Expand All @@ -139,7 +139,7 @@ it('should close the dialog if no group is selected and the Merge button is clic
id: 'yet-another-id',
description: 'Yet another simple task',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
},
Expand Down Expand Up @@ -180,7 +180,7 @@ it('should dispatch the action to merge groups', () => {
id: 'some-id',
description: 'A simple task',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
},
Expand All @@ -191,7 +191,7 @@ it('should dispatch the action to merge groups', () => {
id: 'another-id',
description: 'Another simple task',
completed: false,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
},
Expand All @@ -202,7 +202,7 @@ it('should dispatch the action to merge groups', () => {
id: 'yet-another-id',
description: 'Yet another simple task',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
},
Expand Down
14 changes: 7 additions & 7 deletions packages/com.sncommunity.advanced-checklist/src/features/tasks/NotePreview.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,40 +6,40 @@ const workTasks = [
{
id: 'test-b-1',
description: 'Test #1',
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
{
id: 'test-b-2',
description: 'Test #2',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
]

const personalTasks = [
{
id: 'test-c-1',
description: 'Test #3',
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
{
id: 'test-c-2',
description: 'Test #4',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
]

const miscTasks = [
{
id: 'test-d-1',
description: 'Test #5',
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
{
id: 'test-d-2',
description: 'Test #6',
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
]

Expand Down Expand Up @@ -123,7 +123,7 @@ it('should render a summary of the remaining group(s)', () => {
{
id: 'test-e-1',
description: 'Test #7',
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
sections: DEFAULT_SECTIONS,
Expand Down
10 changes: 5 additions & 5 deletions packages/com.sncommunity.advanced-checklist/src/features/tasks/RenameTaskGroups.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ it('renders the alert dialog with an input box', () => {
id: 'some-id',
description: 'A simple task',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
},
Expand Down Expand Up @@ -54,7 +54,7 @@ it('should dispatch the action to merge groups', () => {
id: 'some-id',
description: 'A simple task',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
},
Expand All @@ -65,7 +65,7 @@ it('should dispatch the action to merge groups', () => {
id: 'another-id',
description: 'Another simple task',
completed: false,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
},
Expand Down Expand Up @@ -118,7 +118,7 @@ it('should dispatch the action to merge groups on Enter press', () => {
id: 'some-id',
description: 'A simple task',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
},
Expand All @@ -129,7 +129,7 @@ it('should dispatch the action to merge groups on Enter press', () => {
id: 'another-id',
description: 'Another simple task',
completed: false,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
},
Expand Down
4 changes: 2 additions & 2 deletions packages/com.sncommunity.advanced-checklist/src/features/tasks/TaskGroup.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ const defaultGroup = {
id: 'test-1',
description: 'Testing',
completed: false,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
{
id: 'test-2',
description: 'Testing',
completed: false,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
sections: DEFAULT_SECTIONS,
Expand Down
2 changes: 1 addition & 1 deletion packages/com.sncommunity.advanced-checklist/src/features/tasks/TaskItem.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const task: TaskModel = {
id: 'test-1',
description: 'Testing #1',
completed: false,
createdAt: new Date(),
createdAt: new Date().toISOString(),
}

it('renders a check box and textarea input', async () => {
Expand Down
8 changes: 4 additions & 4 deletions packages/com.sncommunity.advanced-checklist/src/features/tasks/TaskSectionList.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ const defaultGroup: GroupModel = {
id: 'test-1',
description: 'Testing #1',
completed: false,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
{
id: 'test-2',
description: 'Testing #2',
completed: false,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
sections: [
Expand Down Expand Up @@ -53,7 +53,7 @@ it('renders the completed tasks section', () => {
id: 'test-3',
description: 'Testing #3',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
})

testRender(<TaskSectionList group={groupWithCompletedTask} />)
Expand Down Expand Up @@ -91,7 +91,7 @@ it('renders default sections', () => {
id: 'test-3',
description: 'Testing #3',
completed: true,
createdAt: new Date(),
createdAt: new Date().toISOString(),
},
],
}
Expand Down
Loading