Fixes rectangle deletion on Android. Also changes the way delete works.

[chelseatroy]

• No more transparent button with an x on it that has to move around if a rectangle's edge is near the image's edge.
• Now, in delete mode, all rectangles go from green to red. Click anywhere on a rectangle to delete it.

Fixes #341.

What this change should allow you to do:

So, originally, the way that drawing flows worked was that they had insert mode and delete mode. In insert mode, you could draw and move rectangles. In delete mode, a transparent, round button with a green border and a green x appeared in the upper right hand corner.

Well, if a rectangle was up against the right side or top of the image, you couldn't reach the delete button.

So an open source contributor submitted a change that would move the button to any of the four corners, depending on where the rectangle was.

This solved the problem of making delete touch targets available for all rectangles. But it introduced a new problem: the button was now harder to find. Because it is transparent with a green x, it blends into images that have green in them, or that have a lot of detail in them. Plus, now you could not narrow down where it would be based on the rectangle position, because it could be in any of the four corners.

There was another problem: Android and iOS calculate positions slightly differently, and the change was calibrated for iOS screen dimension units. So on Android it didn't work, since Android thought the button was in a different place than it was.

We could have just added position conversions for all four corners for Android, doubling our number of special cases from 4 to 8. Such code would be a pain to maintain, and the feature already wasn't user-friendly.

So I changed it.

Now, there is no more "remove" button on the rectangles. In insert mode, it's a green rectangle that you can draw and resize, like before.

In delete mode, all the rectangles turn red, and clicking on one removes it.

This way, since at least one corner of the rectangle is always accessible, it is always possible to delete.

Invision Mock-ups:

Review Checklist

• Does it work in Android and iOS?
• Can you rm -rf node_modules/ && npm install and app works as expected?
• Are tests passing?

My goals for this PR:

1. Simplify the code. We had added edge case after edge case for a while, and it took me several days to identify the problem. I would not want another maintainer to have to do this.
2. Make a friendlier UI. As the developer of the app, I often struggled to find the "remove" button. I cannot imagine that volunteers would have an easier time finding it than I did.
3. Fix the Android situation. Rectangles need to be deletable on the app for both frameworks.

[beckyrother]

I really like this solution! Thinking about a11y / colorblindness though, just switching between red and green may not be super obvious for everyone. How do you feel about adding a diagonal line pattern inside the boxes as well, something like: https://projects.invisionapp.com/d/main#/console/13393648/434396833/preview

[chelseatroy]

I really like this solution! Thinking about a11y / colorblindness though, just switching between red and green may not be super obvious for everyone. How do you feel about adding a diagonal line pattern inside the boxes as well, something like: https://projects.invisionapp.com/d/main#/console/13393648/434396833/preview

@beckyrother Oooh, you're right! I forgot to check that. I appear to not have access to that link (no idea why). Could you please post a screenshot of it here?

[beckyrother]

Here's the image:

Is there a different email that you use for InVision? You've got edit permissions with chelsea@zooniverse but I can add another if that's easier

[chelseatroy]

@beckyrother here's one thing I can definitely do:

Let me see what I can find about filling shapes with a pattern!

[chelseatroy]

@beckyrother WAIIIIIT I got patterns going. I need to figure out the math for this specific pattern but I got it!

[chelseatroy]

For reviewers' reference: I timeboxed the pattern effort, hit the timebox, and fell back on the semi-opaque fill solution.

[mcbouslog]

This LGTM!

I noted an unrelated issue (also on master) I'll open a GitHub issue for we discussed on Slack.

[hound]

'height' is not defined no-undef
'width' is not defined no-undef

[mcbouslog]

• for iOS iPhone11 I got an error on drawing workflow load - naturalHeight undefined per MarkableImage.js. Adding the following default props to MarkableImage.js appears to fix:

MarkableImage.defaultProps = {
    subjectDimensions: {
        naturalWidth: 200,
        naturalHeight: 240
    },
}

• on Android Pixel 3 there was still a little cutoff, but changing the height to 260(in DrawingClassifier.js and MarkableImage.js) seems to help - EDIT - just read commit messages, not sure about height, 240 is definitely better, not sure if downsides to increasing to 260 or 280, so I'm going to mark this as ok, but def something to revisit (as already noted)

[mcbouslog]

Looks/works great on iPhone11, Pixel 2, and Pixel 3!