-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add focal point selector to images to allow for some control over sorl cropping. #154
base: develop
Are you sure you want to change the base?
Conversation
@@ -91,6 +91,19 @@ class File(models.Model): | |||
help_text="Text used for screen readers" | |||
) | |||
|
|||
# For images to zoom in on a certain point when cropping/scaling. | |||
focal_x = models.CharField( |
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.
It feels to me that this should be IntegerField or DecimalField, as it will be storing numbers from 0 to 100 - it should immediately throw an exception if you try to put an invalid value in here. (If for technical reasons it does need to be a CharField, 20 seems like a rather large max_length
.)
<style> | ||
#focal-point-pointer { | ||
position: absolute; | ||
top: {% if original.focal_y %}calc( {{ original.focal_y }}% - 5px){% else %}50%{% endif %}; |
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.
I think the {% else %}
needs to be moved to the left a bit - currently if there's no original.focal_y
it will be misplaced by 5 pixels (it should be calc(50% - 5px)
).
})() | ||
</script> | ||
<style> | ||
#focal-point-pointer { |
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.
It's OK to target IDs in Javascript, but it's probably best to use our standard CSS class targeting for CSS.
|
||
//Calculate FocusPoint coordinates | ||
var offsetX = e.pageX - image.getBoundingClientRect().left; | ||
var offsetY = e.pageY - image.getBoundingClientRect().top; |
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.
Are you sure this is not going to miscalculate? mouseEvent.pageY
"returns the Y (vertical) coordinate in pixels of the event relative to the whole document". getBoundingClientRect().top
returns the Y position relative to the viewport.
Relies on a PR for the PT that overrides the render_lazy_image function to pass the crop values.