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

Clean up command factory #3431

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Clean up command factory #3431

wants to merge 4 commits into from

Conversation

john-traas
Copy link
Contributor

@john-traas john-traas commented Feb 4, 2025

This PR adds utility functions that use built in functions from prosemirror to handle operations related to creating/removing and toggling between list types in the text editor.

Tests are also added for this functionality.

Fixes: #3313

Copy link

github-actions bot commented Feb 4, 2025

Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3431/

@john-traas john-traas force-pushed the clean-up-command-factory branch 2 times, most recently from 48cb384 to c6a8a28 Compare February 11, 2025 10:55
@john-traas john-traas marked this pull request as ready for review February 11, 2025 11:05
@john-traas john-traas force-pushed the clean-up-command-factory branch from c6a8a28 to 24b1710 Compare February 11, 2025 13:18
@adrianschmidt adrianschmidt force-pushed the main branch 3 times, most recently from 197298f to a0e1088 Compare February 13, 2025 18:06
Copy link
Contributor

@adrianschmidt adrianschmidt Feb 15, 2025

Choose a reason for hiding this comment

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

Why move tests into a folder called __tests__? Is that a prosemirror convention? Because we don't use that pattern anywhere else in lime-elements… 🧐

Copy link
Contributor Author

@john-traas john-traas Feb 15, 2025

Choose a reason for hiding this comment

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

Ah... The initial folder/file was generated by cursor when I had it help me setup the boilerplate for the tests. We should follow the normal pattern we use and have the tests alongside the files they are testing.

@adrianschmidt
Copy link
Contributor

If you try to make sub-lists, the behaviour quickly gets quite weird 🤔

Screen.Recording.2025-02-15.at.22.05.14.mov

@adrianschmidt adrianschmidt force-pushed the clean-up-command-factory branch from ed54b1f to fba00f3 Compare February 15, 2025 21:08
@john-traas
Copy link
Contributor Author

If you try to make sub-lists, the behaviour quickly gets quite weird 🤔

Screen.Recording.2025-02-15.at.22.05.14.mov

I think I have some ideas of why this is happening. I'll take a look again on Monday.

@adrianschmidt
Copy link
Contributor

adrianschmidt commented Feb 16, 2025

Yeah, I don't expect you to be working on the weekend just because I am 😂 No stress my friend! 🤗

@john-traas john-traas force-pushed the clean-up-command-factory branch from fbe44da to 8eb4802 Compare February 21, 2025 15:08
- instead of trying to convert nested list nodes and creating extra transactions we limit functionality
@john-traas john-traas force-pushed the clean-up-command-factory branch from 8eb4802 to 07b8313 Compare February 21, 2025 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Text editor: switching a list between ordered an unordered does not work
2 participants