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

Add focal point selector to images to allow for some control over sorl cropping. #154

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cms/apps/media/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class FileAdmin(VersionAdmin, SearchAdmin):
'fields': ["title", "file"],
}),
('Media management', {
'fields': ['attribution', 'copyright', 'alt_text', 'labels'],
'fields': ['attribution', 'copyright', 'alt_text', 'labels', 'focal_y', 'focal_x'],
}),
]
filter_horizontal = ['labels']
Expand Down
13 changes: 12 additions & 1 deletion cms/apps/media/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,20 @@ class ImageChangeForm(forms.ModelForm):
required=False,
)

focal_x = forms.CharField(
widget=forms.HiddenInput,
required=False,
)

focal_y = forms.CharField(
widget=forms.HiddenInput,
required=False,
)

class Meta:
model = File
fields = ['changed_image', 'title', 'file', 'attribution', 'copyright', 'alt_text', 'labels']
fields = ['changed_image', 'title', 'file', 'attribution', 'copyright',
'alt_text', 'labels', 'focal_x', 'focal_y']

def save(self, commit=True):
if self.cleaned_data['changed_image']:
Expand Down
25 changes: 25 additions & 0 deletions cms/apps/media/migrations/0009_auto_20180822_1622.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.15 on 2018-08-22 15:22
from __future__ import unicode_literals

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('media', '0008_auto_20180801_1633'),
]

operations = [
migrations.AddField(
model_name='file',
name='focal_x',
field=models.CharField(blank=True, max_length=20, null=True),
),
migrations.AddField(
model_name='file',
name='focal_y',
field=models.CharField(blank=True, max_length=20, null=True),
),
]
13 changes: 13 additions & 0 deletions cms/apps/media/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Copy link
Contributor

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.)

max_length=20,
null=True,
blank=True,
)

focal_y = models.CharField(
max_length=20,
null=True,
blank=True,
)

def get_absolute_url(self):
"""Generates the absolute URL of the image."""
return self.file.url
Expand Down
1 change: 1 addition & 0 deletions cms/apps/media/templates/admin/media/file/change_form.html
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
{% block after_field_sets %}
{% if original.is_image %}
{% include 'admin/media/includes/image-editor.html' %}
{% include 'admin/media/includes/focal-point.html' %}
{% endif %}
{% endblock %}

Expand Down
79 changes: 79 additions & 0 deletions cms/apps/media/templates/admin/media/includes/focal-point.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
<fieldset class="module aligned with-legend ">
<h2 class="legend">Focal point</h2>
<div>Specifying a focal point for an image should prevent that location from being cropped.</div>
<div id="focal-point-clear">Clear</div>

<div id="focal-point-container" style="position: relative; width: fit-content">
<img id="focal-point" style="filter:{% if original.focal_y %}brightness(50%){% else %}brightness(100%){% endif %};" src="{{ original.get_absolute_url }}"/>
<div id="focal-point-pointer"></div>
</div>
<script>
(function () {
document.addEventListener('DOMContentLoaded', function() {
document.getElementById('focal-point').addEventListener('click', function (e) {
var pointer = document.getElementById('focal-point-pointer')
var image = document.getElementById('focal-point')

var imageW = image.offsetWidth;
var imageH = image.offsetHeight;

//Calculate FocusPoint coordinates
var offsetX = e.pageX - image.getBoundingClientRect().left;
var offsetY = e.pageY - image.getBoundingClientRect().top;
Copy link
Contributor

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.


//Calculate CSS Percentages
var percentageX = (offsetX/imageW)*100;
var percentageY = (offsetY/imageH)*100;

pointer.style.left = ((offsetX - (pointer.offsetWidth/2))/imageW)*100 + '%'
pointer.style.top = ((offsetY - (pointer.offsetHeight/2))/imageH)*100 + '%'
pointer.style.display = 'block'
image.style.filter = 'brightness(50%)'

document.getElementById('id_focal_x').value = percentageX
document.getElementById('id_focal_y').value = percentageY
})
document.getElementById('focal-point-clear').addEventListener('click', function (e) {
document.getElementById('id_focal_x').value = ''
document.getElementById('id_focal_y').value = ''
document.getElementById('focal-point').style.filter = 'brightness(100%)'
document.getElementById('focal-point-pointer').style.display = 'none'
})
})
})()
</script>
<style>
#focal-point-pointer {
Copy link
Contributor

@lewiscollard lewiscollard Jun 4, 2019

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.

position: absolute;
top: {% if original.focal_y %}calc( {{ original.focal_y }}% - 5px){% else %}50%{% endif %};
Copy link
Contributor

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)).

left: {% if original.focal_x %}calc( {{ original.focal_x }}% - 5px){% else %}50%{% endif %};

display: {% if original.focal_y %}block{% else %}none{% endif %};
width: 10px;
height: 10px;

border-radius: 50%;
border: 2px solid #fff;
background: #000;
pointer-events: none;
}

#focal-point-clear {
background-color: #d0dbe6;
color: #6f7e95;
border-radius: 4px;
width: 70px;
height: 32px;
line-height: 32px;
text-align: center;
padding: 0 10px;
margin: 12px 0;
cursor: pointer;
transition: background .3s;
}
#focal-point-clear:hover {
background-color: #639af5;
color: #fff;
}
</style>
</fieldset>