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.

Sign up for GitHub

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

Jump to bottom

Implement project-layer annotation UI #4665

Merged
merged 5 commits into from
Feb 22, 2019
Merged

Implement project-layer annotation UI #4665

merged 5 commits into from
Feb 22, 2019

Conversation

alkamin
Copy link
Contributor

@alkamin alkamin commented Feb 20, 2019 •
edited
Loading

Overview

This PR moves our annotation functionality into the project layers UI. There are a few changes made for UI consistency.

Checklist

  • Description of PR is in an appropriate section of the changelog and grouped with similar changes if possible

Demo

image

Notes

I will make an issue for adding annotation stats to the project layer stats endpoint.

Testing Instructions

  • Make annotations using the layer UI for at least 2 layers. Make sure that they annotations do not cross over to the wrong layer, etc.

  • Make annotations using the old UI. Ensure this still works as expected

Closes #4535

@alkamin alkamin force-pushed the feature/ak/project-layer-annotation-ui branch from ff1d067 to 24b383f Compare February 20, 2019 18:26
@alkamin alkamin requested a review from aaronxsu February 20, 2019 18:33
@alkamin alkamin marked this pull request as ready for review February 20, 2019 18:33
aaronxsu
aaronxsu requested changes Feb 20, 2019
View reviewed changes
Copy link
Member

@aaronxsu aaronxsu left a comment

Choose a reason for hiding this comment

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

Works well on v1 and v2 annotation UI for creating, editing, and deleting annotations. A few issues on v2 UI:

  1. There does not seem to have outgoing tile requests for getting scene tiles to be rendered when you switch to annotation tab
  2. If you leave some shapes as Unlabeled and apply filters on them, they don't show up in the filtered list of annotations (v1 UI seems to have the same issue)
  3. Within the three dots option button next to the drop down filter, only the delete all option works, others don't seem to trigger any function calls or route changes

I am ok with dealing with 2 and 3 in this one or creating new cards for them.

@alkamin
Copy link
Contributor Author

alkamin commented Feb 21, 2019

  1. I don't think this should be something the annotation logic should be concerned about. Not that it isn't a problem, but I think it may be a problem with the project layer UI in general and not this set of changes (unless it worked before this PR?)

  2. I'll look into it but if it takes more than a little bit to figure out I'll write up an issue on it instead.

  3. Good catch! Missed those.

@aaronxsu
Copy link
Member

aaronxsu commented Feb 21, 2019 •
edited
Loading

@alkamin per discussions during scrum:

@alkamin
Copy link
Contributor Author

alkamin commented Feb 21, 2019

Agreed on all counts. Probably worth another look now.

@alkamin alkamin mentioned this pull request Feb 21, 2019
Open
aaronxsu
aaronxsu approved these changes Feb 21, 2019
View reviewed changes
Copy link
Member

@aaronxsu aaronxsu left a comment

Choose a reason for hiding this comment

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

Good to go!

@alkamin alkamin merged commit 8dec16f into develop Feb 22, 2019
@aaronxsu aaronxsu deleted the feature/ak/project-layer-annotation-ui branch February 22, 2019 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aaronxsu aaronxsu aaronxsu approved these changes

Assignees

@aaronxsu aaronxsu

Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@alkamin @aaronxsu