-
Notifications
You must be signed in to change notification settings - Fork 2
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
Task/wg 403 create asset geometry component #295
Task/wg 403 create asset geometry component #295
Conversation
overflow-x: hidden; | ||
justify-items: flex-end; | ||
align-items: flex-end; | ||
width: 100%; | ||
} | ||
.metadataTable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Things taken out here were styles that were not being applied by the browser
? turf.bbox(selectedFeature) | ||
: null; | ||
|
||
const geometryType: FeatureType = getFeatureType(selectedFeature); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you want to
A) keep the variable name and then comment that you are considering only geometry types (subset of FeatureType). 🤔
or maybe:
B) const geometryType = selectedFeature.geometry.type
there is a difference currently with angular version, as here if its an something like an image or video feature, we don't show point coordinates (as its featureType would be image
and then not captures by any of the conditional rendering below. but i think would be fixed with approach (B).
(good tests by the way as covers this case well 💯 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I think B works well. This is updated with the latest push
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. 👍 just the issue on video + image point features.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome; LGTM 👍
Overview:
PR Status:
Related Jira tickets:
Summary of Changes:
Testing Steps:
UI Photos:
Polygon:
Line String
Point LOL at the pic
Geometry Collection
Notes: