Fix dirty dots not preserved when moving multiple cells

[Alanhou1222]

References

Fixes #16060
Closes #16161 (includes)

Code changes

In packages/cells/src/model.ts, make the setDirty() function into a set function set isDirty().
In packages/notebook/src/widget.ts, move logics that set new cells dirty into the for loop of the moveCell function and make the code more clear.

User-facing changes

dirty dots are kept even when moving multiple cells

Backwards-incompatible changes

None

[jupyterlab-probot]

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link:

[Alanhou1222]

@krassowski Not sure if it's expected, but moved cells won't remember their content before being moved. Therefore, even if users revert the changes on a moved cell, the cell is still considered dirty.

[krassowski]

but moved cells won't remember their content before being moved

Indeed, undo does not work after moving a cell - good catch! This is not expected, we should open a separate issue to track it and then fix it.

This PR looks good to me on the implementation side (the failing Python tests are unrelated); ideally we would also include unit tests in packages/notebook/test/widget.spec.ts for the dirty dot being preserved. Would you be interested in adding such a unit test (whether in this PR or in another)?

[Alanhou1222]

but moved cells won't remember their content before being moved

Indeed, undo does not work after moving a cell - good catch! This is not expected, we should open a separate issue to track it and then fix it.

This PR looks good to me on the implementation side (the failing Python tests are unrelated); ideally we would also include unit tests in packages/notebook/test/widget.spec.ts for the dirty dot being preserved. Would you be interested in adding such a unit test (whether in this PR or in another)?

Sure, I'll write some unit tests.

[Alanhou1222]

@krassowski I added two tests, one verifies dirty state is preserved after moving one code cell and another verifies dirty states are preserved after moving multiple cells. Let me know if you think there are other situations that should be tested.

[krassowski]

I wonder why is it that rendered and headingCollapsed do not need - n + 1 but isDirty needs it? It seems a bit suspicious to me that the implementation here differs from the one for markdown cell properties. I suspect that one of those is wrong.

[krassowski]

Tested locally, works great. Thank you @Alanhou1222!