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

Text Annotation Tool #105

Merged
merged 32 commits into from
May 18, 2023
Merged

Text Annotation Tool #105

merged 32 commits into from
May 18, 2023

Conversation

nikhiilll
Copy link
Contributor

Text Annotation

Added the text annotation functionality. To implement the text annotation, we are using a ForeignObject inside the annotation SVG. We use a Textarea input to let the user type and get the text. Once the user starts drawing another annotation, we transform the input to a p tag containing the same text and styling that the user had.

Changes

  • annotationContainer.css: Added styling for the textbox functionality and the font size input
  • annotation-tool.js: Added the text annotation tool option, font size input and handling for annotation-specific functions
  • annotation-group.js: Changes to handle the creation and import of text annotation
  • annotation-svg.js: Changes to handle font size change and parsing of text/foreign object nodes
  • base.js: Small changes to handle the export for text annotation
  • text.js: Added this file which contains all the text annotation functionality
  • viewer.js: Implemented functions to implement mouse handlers to handle the positioning of text annotation
  • mview.html: Added the import for the text annotation file

nikhiilll added 19 commits July 13, 2022 16:40
Implemented the four corner resizing of the text input div and movement of the input div.
To do in future commits:
• Switching the text input to SVG text and vice versa.
• Styling of the input div.
• Checking the disable mouse trackers and enabling them.
Added the export functionality for the foreign object.
Also, removed the transition from textarea to SVG text.
Working on switching the multiple annotations to text on switching.
Made changes for the transition to work for multiple input tags which are placed according to user selection.
Added the annotation toolbar input for font-size and implemented the functionality to change the font-size. Also, added relevant comments to document the code.
Fixed the bug where the annotation is not rendered during import, if the annotation exists after the textbox annotation. Cloned the node instead of using the imported node while adding to the annotation SVG.
1. The text color now takes the stroke color at the start and also changes when the stroke color is changed using color picker.
2. Fixed the spacing issue and text moving while switching from text to p tag after transformation.
Implemented the horizontal resizing functionality. Fixed the text wrapping depending on width of the textbox.
1. The text annotation now gets highlighted and unhighlighted on hover.
2. All the group annotations get highlighted on hover and vice versa.
3. Fixed the code for erase of text annotation.

Facing some bugs with the annotation not changing with the toolbar in the code causing the erase functionality to malfunction on the first click.
1. The highlight functionality is now added, it is working end-to-end along with other annotations in the group.
2. Fixed the bug where the erase functionality would work for all the modes. Now it only works in the erase mode.
3. Fixed the mousetracker bug where the trackers for the annotations were not getting destroyed correctly.
1. Implemented the absolute mapping for the font-size
2. Completed the changes for the textbox sizing to be relative instead of absolute for attributes like spacing, width, padding, resizer size etc.
1. Implemented the text size functionality taking into account the zoom level.
2. Bug fixes for font change
Implemented the new font size input with the following features:
1. Increase/decrease buttons for font size
2. Input area in between the buttons to type the font size
3. Dropdown with predefined values for the font size
@nikhiilll nikhiilll requested a review from RFSH April 10, 2023 20:47
@nikhiilll nikhiilll self-assigned this Apr 10, 2023
This was linked to issues Apr 10, 2023
js/viewer/viewer.js Outdated Show resolved Hide resolved
js/toolbar/annotation-tool.js Outdated Show resolved Hide resolved
js/toolbar/annotation-tool.js Show resolved Hide resolved
js/viewer/annotation/text.js Show resolved Hide resolved
js/viewer/annotation/text.js Outdated Show resolved Hide resolved
js/viewer/annotation/text.js Show resolved Hide resolved
js/viewer/annotation/text.js Show resolved Hide resolved
js/viewer/annotation/text.js Show resolved Hide resolved
js/viewer/viewer.js Show resolved Hide resolved
js/viewer/viewer.js Outdated Show resolved Hide resolved
Copy link
Member

@RFSH RFSH left a comment

Choose a reason for hiding this comment

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

Apart from the comments,

  • Did you apply the changes to make sure this feature is working in Safari? If you did, please comment in places that you had to change the implementation why the other method didn't work.
  • Let me know you've gone through the viewer regression tests for Firefox, Chrome, and Safari. I'll do the same thing as the last step to make sure everything is good to go.

js/toolbar/annotation-tool.js Show resolved Hide resolved
js/viewer/annotation/text.js Show resolved Hide resolved
js/viewer/annotation/text.js Outdated Show resolved Hide resolved
js/viewer/annotation/text.js Show resolved Hide resolved
js/viewer/annotation/text.js Outdated Show resolved Hide resolved
textInput.style.overflowY = "hidden";
textInput.style.fontSize = this._attrs["font-size"] + "px";
textOuterDiv.style.padding = (this.parent.getStrokeScale() * this._ratio * 4) + "px";
textOuterDiv.style.border = (this.parent.getStrokeScale() * this._ratio) + "px solid red";
Copy link
Member

Choose a reason for hiding this comment

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

Add a TODO about this line and briefly explain how you would go about implementing it. You should also create a new issue about this.

Removed the text SVG element that gets created while initializing the text annotation. As we use a foreignObject with input or p tag, we do not need the SVG text tag.
nikhiilll and others added 9 commits May 10, 2023 12:57
1. Fixed th empty text annotation being exported and rendered after saving the text annotation.
2. The annotation list would show wrong color for the group, fixed now.
- discard wasn't properly removing the drawn group. so in the drawn svg
  you could see duplicated groups.

- when we were adding the saved state back, the new elements were DOM
  elements and not SVG. while this was fine for the elements that we had,
  foreignObject wasn't working properly because of it.
Safari does not work well with the logic we have for positioning the resize control on the right border. Made temporary fix to add the resize functionality only for non-safari browsers for now.
instead of changing the styles of e.target, I'm now always changing
the styles of divCount. e.target could be the textarea and as a result
we were resizing the textarea instead of moving it around.
- avoid using both style and attributes for foreignObject
  - was causing issues in safari
  - it's also cleaner this way
- add a new parameter in addAnnotation instead of abusing the
  existing params.
- clean up the code
- avoid adding id to the imported svg
- make the class names consistent
- remove commented code
@RFSH
Copy link
Member

RFSH commented May 18, 2023

In Safari, we couldn't figure out how to show the handler for resizing the textbox, so for now, that feature is not used in Safari.

Everything else seems to be working as expected. I will merge this PR and get back to the Safari issue later.

@RFSH RFSH merged commit aba334f into master May 18, 2023
@RFSH RFSH deleted the text-annotation-tool branch May 18, 2023 01:37
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.

Font size logic for text drawing Add text box drawing tool
2 participants