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

UI drawing enhancements #5

Closed
wants to merge 7 commits into from

Conversation

mwomick
Copy link
Contributor

@mwomick mwomick commented Aug 10, 2024

Description

I noticed there were a couple unexpected behaviors of this app and some missing features that are ubiquitous among similar apps--annotation products, vector graphics editors, presentation software, etc.

Unexpected behaviors included:

  • Polygons not closing when a new point is added within the radius of the first node representing the first point in a polygon.
  • Ghost edges (between n-1th point and cursor position at time of shape closure) sticking to the screen until a refresh is called.
  • Lines not capping (i.e. with an arc) until a new line is started.
  • Unexpected overflow in the copyable outputs (NumPy, JSON) caused the page content to significantly shift.
  • Several logic issues that caused things like color repetition after 5 or 6 lines are drawn.
  • Unexpected scroll direction for zoom in (cc-wise scroll instead of c-wise) and simultaneous scrolling and zooming

Missing features included:

  • The ability to undo points. This is an important feature because otherwise a single mistake can cause the user to start over completely.
  • The ability to constrain angles to 45 degree increments. This is useful, for example, in some common special cases where many of the lines in an image are vertical or horizontal.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

How has this change been tested, please provide a testcase or example of how you tested the change?

Since the changes were mostly UI-related, the changes were tested by using the web app in Safari and Chrome. For example, after I added the undo feature, I tested it by trying to draw a polygon and undoing point additions until the shape disappeared and then re-drawing and re-undoing the polygon to test for consistency. I also added lines (not undoable at this point) and then polygons and then tested to make sure switching modes (line/polygon) did not affect the feature.

Any specific deployment considerations

Since this is a static web page and is deployed on GitHub, it might be worth considering that the code is readable and the comments in the code are viewable by the end user. Since the code base is small, this is negligible on performance (i.e. content delivery), but it might be a cause of concern depending on Roboflow's expectation for production polish and style.

@mwomick
Copy link
Contributor Author

mwomick commented Aug 17, 2024

It seems that #10 and #11 are doing some of these things and getting attention, I would still like to have my pull request merged, if appropriate.

@capjamesg @yeldarby

@capjamesg
Copy link
Collaborator

@mwomick Thank you for your contribution! There are a few merge conflicts that are hard for me to address as a reviewer. Would you be able to merge your contributions with main on this branch? I am happy to take a look at your PR and merge it if everything works as expected. It looks like you have a lot of exciting improvements in your code.

@mwomick
Copy link
Contributor Author

mwomick commented Aug 23, 2024

@capjamesg I just made a new pull request (#12) where I updated my code according to the most recent changes.

@capjamesg
Copy link
Collaborator

I am closing this as your new PR has been merged.

@capjamesg capjamesg closed this Aug 23, 2024
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.

2 participants