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

feat: freeform drawing and annotation support to image occlusion #2983

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

krmanik
Copy link
Contributor

@krmanik krmanik commented Jan 31, 2024

Added support for freeform drawing and annotation. The freeform data is stored in image field as svg. When editing the svg data parsed as canvas object then again saved as svg in image field. The freedraw image is second layer hidden below occlusion and above original image.

When path tool selected freedraw enabled and these data are stored as string in image field. When annotation tool, in topbar selected, all the shapes are drawn as annotation with semi-transparent color with isAnnotation attribute added to fabric object for that shape. Also Path class added to support type, it is not added to cloze occlusion field. The data are stored in image field.

I have tried to add annotation support for shapes other than path using cloze ordinal 0, it works when added but after editing some card removed (also tried ia=1 is annotation) . @dae @abdnh, I need help on this issues.

@glutanimate
Copy link
Contributor

Thanks, Mani, this is looking quite nice already!

Some initial feedback, just focused on editor UX for now, while we're still discussing other parts of the implementation in #2980:

  • Tool properties: I would recommend that we use a similar interaction pattern to Notability here and only show the properties pop-up upon clicking/tapping on the tool for the second time. I.e.: Only show the pop-up if the tool is already active and the user clicks/taps on it for a second time. That way users can quickly switch between tools without invoking the pop-up on every switch. Changing the color/brush is likely something they won't do as often as switching between tools
  • Color indicator: I would suggest that we move this directly below the tool icon, similar to how we do it in the note editor for the text background color, i.e.: Screenshot_20240212_005344
    Right now the indicator blends too much into the button border, IMHO, and I think it would be easier to parse if there was a clear visual separation between the indicator and button border. An alternative option might be to color the tool icon itself in the selected color, but visibility in dark mode might become a problem in that case.
  • Color visibility: The white option in the properties pop-up is quite hard to see at the moment. Perhaps we could boost visibility a bit by drawing a thin grey border around it.
  • Support for grouping: Grouped paths are currently dropped when saving the image. I think that grouping is something that users will likely also expect to be able to do for annotations as it e.g. allows to combine multiple lines of handwritten text together and easily move it around. One case we need to guard against with this, though, is users attempting to group annotations together with masks. It would be good to prevent this in the first place.
  • Support for undoing: I'm sure you already had this on your scope, but it would be nice to also extend the undo/redo system to annotations.

@krmanik
Copy link
Contributor Author

krmanik commented Feb 12, 2024

The other UX will be fixed in next commit, but undo redo very slow on path tool, I will check again for it.

@krmanik krmanik marked this pull request as draft April 12, 2024 02:07
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.

None yet

2 participants