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(region): Add region manager logic and renderers #406

Merged
merged 3 commits into from
Mar 31, 2020

Conversation

jstoffan
Copy link
Collaborator

No description provided.

@jstoffan jstoffan requested a review from a team as a code owner March 31, 2020 01:02
@jstoffan jstoffan force-pushed the add-region-manager branch from c2ac662 to 7eab2e0 Compare March 31, 2020 02:34
src/common/BaseAnnotator.ts Show resolved Hide resolved
onSelect(annotationId);
};
const handleKeyPress = (event: React.KeyboardEvent): void => {
if (event.key !== KEYS.enter) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we include space too?

Comment on lines +4 to +5
&:focus,
&:hover {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do focus and hover also need the thicker stroke width?


format({ annotations = [], scale = 1 }: Props): Annotation[] {
return annotations
.filter(annotation => annotation.type.indexOf('region') >= 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

=== vs indexOf? Also could destructure type out of annotation

Copy link
Contributor

@ConradJChan ConradJChan left a comment

Choose a reason for hiding this comment

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

LGTM!

@jstoffan jstoffan merged commit 4772d26 into box:master Mar 31, 2020
@jstoffan jstoffan deleted the add-region-manager branch March 31, 2020 21:18
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