-
Notifications
You must be signed in to change notification settings - Fork 5
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
Minor improvments: local images and ability to edit and duplicate cards #6
base: main
Are you sure you want to change the base?
Conversation
Thanks for bringing these up. Are these changes something you would want merged into the main repo, or are they something you need just for yourself in your fork? Regardless, I'm interested in your use cases for them, if you're willing to share :) If you want these merged into the main repo, then at least the card editing and creation/duplication needs to be put behind a setting, similar to |
First of all thank, you for this (free software) sorting card tool. My idea was to share with you some modifications I made. You may want to merge them in your repo. The uses cases are very common:
I will explore how to add settings. |
Ah, thanks for the clarifications. Since duplicating cards has a different purpose than editing cards, my earlier suggestion to place them behind a single settings flag might not be a good idea. It could be that there is no card editing phase, but the host still wants to allow duplicating cards. |
If you do edit the settings, there's one piece of complexity that maybe in hindsight wasn't the best idea. The settings flags are packed into the bits of a single number. https://github.com/indigane/cardsort/blob/main/common-scripts.js#L3-L6 const settingsFlagsEnum = {
allowCategoryEditing: 1,
isRandomized: 2,
}; So when adding flags, these need to be powers of 2, for example: const settingsFlagsEnum = {
allowCategoryEditing: 1,
isRandomized: 2,
allowCardEditing: 4,
allowCardDuplication: 8,
}; |
I added two settings, I hope it is OK. |
Some final improvements in order to show the old label when it has been edited (it also work with Export and Share). I think my work on this feature is over. |
Hi, Do you agree to merge this PR? I have some other changes to propose as a PR. It would be easier to submit them once this first PR has been accepted or rejected. |
No description provided.