Problem/Motivation

The focal point module expects the image field widget to be of the 'image' type. When using entity browser the widget needs to be changed and that stops this focal point from working with images placed in this way.

Proposed resolution

Integrate focal point with file entity so that after selecting an image through entity browser the user/editor can edit the file entity where they will see the focal point preview widget in the file entity modal dialog, allowing them to select an appropriate focal point.

Steps for testing

  • Enable file entity module
  • Create a content-type, attach a file field (cardinality: unlimited)
  • Go to "Manage form display", select "Editable file" as field widget
  • Create a node and upload a image, click on the "Edit" button next to the uploaded file
  • The file entity overlay should display a preview of the image, the focal point widget and a focal point preview link
  • Set focal point
  • Submit the overlay, save the node
  • View the node

Remaining tasks

More testing by users to confirm this works with the 'image' widget as well as the 'entity browser' and 'editable file' widgets.

User interface changes

A focal point provided thumbnail with crosshair is displayed in a file entity modal dialog where a user can make their focal point selection and also preview the results of their selection. This is also displayed when editing a file through /admin/content/files.

Original report by Andreas Speck

We are using File Entity Browser to populate image fields. I am looking for a way to nevertheless use Focal Point, but how?

From Focal Point's README, I gather I need to set the Focal Point as the image widget on the from display settings page - which I can't do if I want to use File Entity Browser to allow for uploading and selection of existing image files.

Is there a way to nevertheless use Focal Point to crop and scale the image only for this one instance where it is used (and different for another instance)? How would I go about that?

CommentFileSizeAuthor
#76 integrate_focal_point-2701725-76.patch25.45 KBsamuelpodina
#75 integrate_focal_point-2701725-75.patch14.12 KBreszli
#73 integrate_focal_point-2701725-73.patch14.13 KBsandzel
#69 integrate_focal_point-2701725-69.patch14.14 KBrudins
#64 media-source.png5.27 KBeigentor
#58 integrate_focal_point-2701725-58-interdiff.txt597 bytestanc
#58 integrate_focal_point-2701725-58.patch13.72 KBtanc
#57 integrate_focal_point-2701725-57-interdiff.txt555 bytesundertext
#57 integrate_focal_point-2701725-57.patch13.54 KBundertext
#53 integrate_focal_point-2701725-53.patch13.52 KBbleen
#50 2701725-50.patch11.37 KBwebflo
#48 2701725-48.patch12.43 KBwebflo
#41 focal-point-preview.jpg67.83 KBtanc
#41 file-entity-modal.jpg43.34 KBtanc
#37 interdiff-2701725-36-37.txt2.09 KBtanc
#37 how_to_use_focal_point-2701725-37.patch15.56 KBtanc
#36 how_to_use_focal_point-2701725-36.patch15.33 KBtanc
#32 how_to_use_focal_point-2701725-32.patch15.39 KBandreasderijcke
#31 focal_point_warning_stack_trace.txt6.67 KBandreasderijcke
#25 how_to_use_focal_point-2701725-25.patch14.38 KBnerdacus
#4 file_entity_integration-2701725-4.patch3.38 KBEwout Goosmann
#7 2701725-7.patch6.85 KBwebflo
#8 2701725-8.patch6.78 KBwebflo
#9 2701725-9.patch8 KBwebflo
#15 2701725-15.patch9.89 KBwebflo
#17 focal_point-file_entity_browser_integration-2701725-17.patch9.76 KBnerdacus
#19 focal_point-file_entity_browser_integration-2701725-19.patch9.44 KBnerdacus
#20 focal_point-file_entity_browser_integration-2701725-20.patch8.89 KBnerdacus
#22 how_to_use_focal_point-2701725-22.patch14.38 KBbleen
#22 interdiff.txt13.71 KBbleen
#23 how_to_use_focal_point-2701725-23.patch14.91 KBnerdacus
#23 interdiff.txt648 bytesnerdacus
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

Andreas Speck created an issue. See original summary.

pminf’s picture

Any update on this? I'm also "forced" to use File Entity Browser because with the standard image widget I'm not able to change file entity fields.

I think we have to alter the form but I don't know which form render array keys I have to set or update.

bleen’s picture

Currently there is no support for the File Entity Browser but its certainly something I would consider. Patches welcome...

Is there a way to nevertheless use Focal Point to crop and scale the image only for this one instance where it is used (and different for another instance)? How would I go about that?

As for this, the short answer is no. The focal point position is tied directly to the file independent of the context of where that file might be displayed. In my opinion, this is the correct behavior.

Ewout Goosmann’s picture

This patch integrates focal point into the file entity module. You can use focal point on the file entity edit form.

Anonymous’s picture

Thanks @ewout-goosmann. This seems to work, so that's good. But it is not very practical in terms of workflow, as you do not set the focal point when you upload an image using an entity browser. So if someone uploads an image when adding or editing content, they would then need to locate the image via /admin/content/files to set the focal point... Hmmm.

The problem might also be that focal_point saves the focal point per image instance, and not separately for each usage of an image.

keboca’s picture

Hey @ewout-goosmann,
Looks like this patch needs to be an update,
Fatal error: Class 'FocalPoint' not found in /var/www/drupalvm/drupal/modules/contrib/focal_point/focal_point.module on line 58

Right now, Focal point is using services to manage all functionality

webflo’s picture

Status: Active » Needs review
FileSize
6.85 KB

Rerolled the patch from #4. This provides the focal point widget on file edit and file edit inline forms.

webflo’s picture

+++ b/focal_point.module
@@ -11,6 +11,9 @@
+use Drupal\focal_point\Plugin\Field\FieldWidget\FocalPointImageWidget;

Not used.

webflo’s picture

Added the preview link to the form.

yoruvo’s picture

Some seriously good work. I love that there's a preview link.

  • I could not apply the patch to the current focal_point version (8.x-1.0-beta3); one of the hunks failed. I had to manually patch some stuff in.
  • I would welcome a less dorky icon for the preview, something monochrome and Drupal 8-y. (This is seriously minor though.)

I almost set it to "reviewed by the community", but not being able to directly apply the patch is a bit of a problem, I guess.

bleen’s picture

Can someone give me the exact steps they are using to test (I dont really use the file entity browser).

I almost set it to "reviewed by the community", but not being able to directly apply the patch is a bit of a problem, I guess.

Once I have steps, I will try to apply to HEAD which is really what is important

I would welcome a less dorky icon for the preview, something monochrome and Drupal 8-y. (This is seriously minor though.)

Please open a new issue for this and I'm open to replacing it...

webflo’s picture

Here some steps for testing.

  • Enable file entity module
  • Create a content-type, attach a file field (cardinality: unlimited)
  • Go to "Manage form display", select "Editable file" as field widget
  • Create a node and upload a image, click on the "Edit" button next to the uploaded file
  • The file entity overlay should display a preview of the image, the focal point widget and a focal point preview link
  • Set focal point
  • Submit the overlay, save the node
  • View the node

The file edit form should display the focal point widget as well. Go to "admin/content/files", edit a file/image. The edit page (files/(fid)/edit) should show the focal point widget.

bleen’s picture

Status: Needs review » Needs work

Generally this looks ok and I'm certainly willing to add this functionality, but I have a few issues:

  • The preview link doesnt work on new nodes (only when you are editing)... see below for a link that explains why...
  • There is a lot of unnecessary code duplication

Here is a more detailed review:

  1. +++ b/focal_point.module
    @@ -63,3 +67,125 @@ function focal_point_field_widget_form_alter(&$element, FormStateInterface $form
    +    'indicator' => array(
    +      '#theme_wrappers' => array('container'),
    +      '#attributes' => array(
    +        'class' => array('focal-point-indicator'),
    +        'data-selector' => $element_selector,
    +        'data-field-name' => 'file-entity',
    +      ),
    +      '#markup' => '',
    

    Can we find a way to abstract the code that is used in Drupal\focal_point\Plugin\Field\FieldWidget:process so we don't duplicate the code needed to build the indicator form element?

  2. +++ b/focal_point.module
    @@ -63,3 +67,125 @@ function focal_point_field_widget_form_alter(&$element, FormStateInterface $form
    +  $display_preview_link = \Drupal::config('focal_point.preview')->get('display_link');
    +
    +  // Even for image fields with a cardinality higher than 1 the correct fid
    +  // can always be found in $item['fids'][0].
    +  $fid = $file->id();
    +  if ($display_preview_link && !empty($fid)) {
    +    // Replace comma (,) with an x to make javascript handling easier.
    +    $preview_focal_point_value = str_replace(',', 'x', $default_focal_point_value);
    +
    +    $preview_link = [
    +      '#type' => 'link',
    +      '#title' => t('Preview'),
    +      '#url' => new Url('focal_point.preview',
    +        [
    +          'fid' => $fid,
    +          'focal_point_value' => $preview_focal_point_value,
    +        ],
    +        [
    +          'query' => array('focal_point_token' => FocalPointImageWidget::getPreviewToken()),
    +        ]),
    +      '#attributes' => [
    +        'class' => array('focal-point-preview-link'),
    +        'data-selector' => $element_selector,
    +        'data-field-name' => 'file-entity',
    +        'target' => '_blank',
    +      ],
    +    ];
    +
    +    $form['preview']['preview_link'] = $preview_link;
    

    Similarly we need to abstract the logic to create a preview link from Drupal\focal_point\Plugin\Field\FieldWidget:process so we are not duplicating code. Also, as it stands the preview link does not work until you have saved the node. See #2790809: Preview page return "Access Denied" when creating a new node

  3. +++ b/focal_point.module
    @@ -63,3 +67,125 @@ function focal_point_field_widget_form_alter(&$element, FormStateInterface $form
    +  $form['focal_point'] = array(
    +    '#type' => 'textfield',
    +    '#title' => 'Focal point',
    +    '#description' => new TranslatableMarkup('Specify the focus of this image in the form "leftoffset,topoffset" where offsets are in percents. Ex: 25,75'),
    +    '#default_value' => $default_focal_point_value,
    +    '#element_validate' => array('\Drupal\focal_point\Plugin\Field\FieldWidget\FocalPointImageWidget::validateFocalPoint'),
    +    '#attributes' => array(
    +      'class' => array('focal-point', $element_selector),
    +      'data-selector' => $element_selector,
    +      'data-field-name' => 'file-entity',
    +    ),
    +    '#attached' => array(
    +      'library' => array('focal_point/drupal.focal_point'),
    +    ),
    +  );
    

    same...

  4. +++ b/focal_point.module
    @@ -63,3 +67,125 @@ function focal_point_field_widget_form_alter(&$element, FormStateInterface $form
    +function focal_point_form_file_image_inline_edit_form_alter(&$form, \Drupal\Core\Form\FormStateInterface $form_state) {
    ...
    +function focal_point_form_file_image_inline_edit_form_submit(&$form, \Drupal\Core\Form\FormStateInterface $form_state) {
    

    nit: needs docblock

  5. +++ b/src/FocalPointHelper.php
    @@ -0,0 +1,32 @@
    +class FocalPointHelper {
    

    Is there a reason why you created a new class here as opposed to adding this to focal_point.manager? I'm honestly not sure where the line should be drawn here...

webflo’s picture

webflo’s picture

Status: Needs work » Needs review
FileSize
9.89 KB

A extra form element is not an easy task because of the ImageWidget inheritance. I tried to refactor the previous patch instead.

webflo’s picture

+++ b/src/Plugin/Field/FieldWidget/FocalPointImageWidget.php
@@ -46,7 +65,6 @@ class FocalPointImageWidget extends ImageWidget {
-          'data-delta' => $element['#delta'],

@@ -82,7 +97,6 @@ class FocalPointImageWidget extends ImageWidget {
-            'data-field-name' => $element['#field_name'],

@@ -104,12 +118,11 @@ class FocalPointImageWidget extends ImageWidget {
-        'data-field-name' => $element['#field_name'],

Can remove the data attributes? I could not find any usages.

nerdacus’s picture

Re-rolled the #15 patch to remove any mentions of the FocalPointHelper. I didn't make any changes with respect to the comments in #16, but I'd like to defend the data attributes as they might be used by quickedit or similar.
So far this integration is looking and working great.

bleen’s picture

+++ b/src/Plugin/Field/FieldWidget/FocalPointImageWidget.php
@@ -46,7 +64,6 @@ public static function process($element, FormStateInterface $form_state, $form)
-          'data-delta' => $element['#delta'],

@@ -82,7 +96,6 @@ public static function process($element, FormStateInterface $form_state, $form)
-            'data-field-name' => $element['#field_name'],

why are these lines being removed?

nerdacus’s picture

Ok, I just wanted one person to protest the removal. Re-rolled the patch without removing the data attributes.

The last submitted patch, 19: focal_point-file_entity_browser_integration-2701725-19.patch, failed testing.

bleen’s picture

I made some small changes to the patch but there are two big ones that need to be looked at:

  • I broke the attachFocalPoint method into several smaller methods; one each for building the focal point indicator form element, the focal point field, and the preview link.
  • I removed the width and height params from getFocalPointFromFile. In the contexts in which this method is called, these params were ultimately not needed.
nerdacus’s picture

I like those changes, and the clean up, but a missing self:: slipped through. After fixing it, the widget still works as designed.

The last submitted patch, 23: how_to_use_focal_point-2701725-23.patch, failed testing.

nerdacus’s picture

Ugh, my bad, funky git history.
This is just an edit of #22.

nerdacus’s picture

PapaGrande’s picture

I'm not seeing the focal point widget when using entity browser. I'm using file_entity (8.x-2.0-beta2) and entity_browser (8.x-1.0-alpha9+11-dev (2016-10-05)) along with patch #25 applied to focal_point (8.x-1.0-beta3+2-dev (2016-09-28)). The field is set to use the "browser for files (modal)". Am I missing a configuration step?

nerdacus’s picture

@PapaGrande, the Focal Point is set when editing the file after selecting & using it via File Browser. Does the Focal Point not appear there for you?

wwhurley’s picture

In Drupal 8.0 and 8.1 the File Entity does not have an edit form. So the code in FileBrowserWidget.php that has:

if (!$entity->getEntityType()->getFormClass('edit')) {
  $can_edit = FALSE;
}

disabled the "Edit" button. Though disabling that section of code allowing the Edit button to show causes an error due to the edit form not being accessible.

bleen’s picture

I can confirm that there is no edit button any more when I test the patch from #25 using simpleytest.me. The drupal version that is installed there is 8.1.11-dev

andreasderijcke’s picture

Tested in Drupal 8.2.2 with

  • focal_point 8.x-1.0-beta3
  • file_entity: 8.x-2.0-beta3
  • entity_browser: 8.x-1.0-alpha10
  • file_browser: 8.x-1.0-alpha1

Functionality works as described.

Only following warnings appear when editing a file, either as message or in dblog:

  1. Notice: Undefined index: #delta in Drupal\focal_point\Plugin\Field\FieldWidget\FocalPointImageWidget::buildIndicatorFormElement() (line 179 of modules/contrib/focal_point/src/Plugin/Field/FieldWidget/FocalPointImageWidget.php).
  2. Notice: Undefined index: #field_name in Drupal\focal_point\Plugin\Field\FieldWidget\FocalPointImageWidget::buildPreviewLinkFormElement() (line 223 of modules/contrib/focal_point/src/Plugin/Field/FieldWidget/FocalPointImageWidget.php).
  3. Notice: Undefined index: #field_name in Drupal\focal_point\Plugin\Field\FieldWidget\FocalPointImageWidget::buildFocalPointFormElement() (line 254 of modules/contrib/focal_point/src/Plugin/Field/FieldWidget/FocalPointImageWidget.php).

Full stack trace in attached txt.

It seems both corresponding data attributes aren't really necessary in the content of a file entity, but i might be missing something.

andreasderijcke’s picture

I forgot to mention in #31 that for Drupal 8.2.2 patch from #25 couldn't be applied correctly.
Attached the corrected patch.

Status: Needs review » Needs work

The last submitted patch, 32: how_to_use_focal_point-2701725-32.patch, failed testing.

weseze’s picture

I could not apply this patch against latest dev, doing it manually did work. (but is a lot of work)

After applying the patch, the focal selection did not appear. Change the file browser from modal to inline didn't work either.

I tried fixing it, but don't have enough time right now to figure out what is going wrong...

stillworks’s picture

Any news on this matter?

tanc’s picture

Status: Needs work » Needs review
FileSize
15.33 KB

Here is a re-roll of #32 against latest dev. In my testing this works nicely. Note that the integration isn't really with the entity browser as in the title of this issue, instead its with file entity. So you need to edit the file to see the focal point preview tool.

tanc’s picture

Updated patch makes the 'data-delta' and 'data-field-name' attributes optional, depending on whether the corresponding array elements exist in $element. Stops the notice errors. Patch and interdiff attached.

stillworks’s picture

Won't work for me. Applied the patch to the focal_point folder, patch is applied without errors, image styles with focal point scale and crop are assigned but not visible.

tanc’s picture

So you've edited an uploaded file_entity based image and set a new focal point? And the image style that you're applying on output uses the focal point scale and crop? You've done a hard refresh of the page and confirmed in the source that the correct image style is being applied?

stillworks’s picture

Yes to all. I switched back and forth from Entity Browser to Image in the Manage form display tab of my fields and the normal image works, I get the little cross to set my focal point. File Entity Browser doesn't show it.

tanc’s picture

Issue summary: View changes
FileSize
43.34 KB
67.83 KB

The file entity browser won't show the preview with focus point cross hair. Instead its the modal dialog that you see after you've chosen an image using entity browser and then clicked edit. That modal is provided by file_entity module and this issue is about integration with that (despite the current title!).

tanc’s picture

Title: How to use Focal_Point with File Entity Browser? » Integrate focal point with file entity and entity browser
Category: Support request » Feature request
Issue summary: View changes

Updated the title and issue summary to better describe the functionality in the patches.

stillworks’s picture

Allright. I have file_entity and entity_browser running works so far. I could a Credit Text field, so i figured out where to edit everything. I added a focal point crop to a image style an set that image style to be used in the edit modal, but how do I get the little image preview you have?

tanc’s picture

You don't need the "Credit Text" field, thats just something I've got on the file entity for that site.

You should get the image preview if you've applied the patch.

stillworks’s picture

Yes I know, but to figure out where something is and what it does. I re-applied the patch, cleared everything and it works now. Thank you for the help!

stillworks’s picture

On another project I get "Reversed (or previously applied) patch detected! Assume -R? [n]" with the latest dev release, freshly downloaded. Applying it anyway breaks the ajax modal.

If I run with no

focal_point.info.yml.rej:

***************
*** 1,11 ****
  name: Focal Point
  type: module
  description: Allows users to specify the focal point of an image for use during cropping.
- core: 8.x
  package: Images
- version: VERSION
  test_dependencies:
    - crop
  dependencies:
    - image
    - crop (>=8.x-1.0-alpha2)
--- 1,17 ----
  name: Focal Point
  type: module
  description: Allows users to specify the focal point of an image for use during cropping.
+ # core: 8.x
  package: Images
+ # version: VERSION
  test_dependencies:
    - crop
  dependencies:
    - image
    - crop (>=8.x-1.0-alpha2)
+ 
+ # Information added by Drupal.org packaging script on 2017-02-01
+ version: '8.x-1.0-beta4+10-dev'
+ core: '8.x'
+ project: 'focal_point'
+ datestamp: 1485910088
gmaxwelled’s picture

This works for me. It would be good to see this included in a new release.

webflo’s picture

Status: Needs review » Needs work

The last submitted patch, 48: 2701725-48.patch, failed testing.

webflo’s picture

webflo’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 50: 2701725-50.patch, failed testing.

bleen’s picture

Chasing HEAD (this patch is untested)

bleen’s picture

Status: Needs work » Needs review

triggering testbot

bleen’s picture

Issue summary: View changes
tanc’s picture

Status: Needs review » Needs work

While the patch applies, I think the functionality is now broken. When clicking the edit button the modal now pops open but is empty. I think this is due to attachFocalPoint in focal_point_form_file_image_edit_form_alter(). The attachFocalPoint is expecting the image element object as its first parameter but instead is being passed the file entity edit form.

undertext’s picture

The main problem is that attachFocalPoint() returns nothing xD

This patch should fix empty popup, but attaching logic should be still updated as I get notices in the logs.

tanc’s picture

Ha! Yes, it should return an array according to the comment block.

This does allow the form to work, but as you say there are notice errors. These are because attachFocalPoint is expecting the form elements of a standard widget. The attached patch is a total kludge just to get around the notice errors but I don't like it at all. It seems to me that it would be good to work on #2657592: Convert focal point selector tool into a standalone form element so that it can be included neatly on the file entity form.

Dennis Cohn’s picture

I've applied #58.
But when I click on the edit button, I don't see a focal point. I can only replace the image..

EDIT: I'm only seeing the focal point on newly uploaded files...
EDIT2: After running cron, clearing the cache a few time, the focal point is also visible at older uploaded files!

kumkum29’s picture

Hello,

use Focal Point with entity browser is a useful functionnality. Do you think add the above patch in the next version this module ?

Thanks.

bleen’s picture

@kumkum29 ... Looks like there is still work to do on this. Any help you can give would be a great help in moving this along.

bleen’s picture

now that the media module is in core ... can someone re-state the steps here. It seems like focal point works out of the box with media based on how its been implemented in core. Am I missing something (likely)?

samtny’s picture

I can confirm Focal Point works out-of-the-box with core Media Entity. The focal point may be set in the popup / entity edit form, and is correctly represented in Image Styles that use the Focal Point steps.

I suggest this ticket should no longer block a release of Focal Point for D8, as File Entity has AFAIK been deprecated anyway for at least a year.

eigentor’s picture

FileSize
5.27 KB

Alright, can also confirm this works with core media Entity.

  1. Create a new media entity, give it a name to your liking (How about "Focal Point Image).
  2. As "media source" select "Image"
  3. On the Form Display for this media entity, select "Focal point" for the image field
  4. In the content type, add a Media reference field and select the media type you created
  5. For this reference field, select Inline Entity Form as widget in the form display
  6. It now shows the Focal point widget after uploading an image

I guess it is not yet usable with entity browser.

AaronBauman’s picture

Is there a thread open already about compatibility with media_library module, which is in core-experimental now?
If not, i will open a new one.

bleen’s picture

@aaronbauman ... I think we need a new issue for media library (which should frankly get more of the love since it will supersede entity browser for images)

AaronBauman’s picture

bleen’s picture

Is it fair to close tis now that we have integrated with media_library?

rudins’s picture

There are still many of us who still haven't migrated to core media. So, here is patch for us.

bleen’s picture

Status: Needs work » Closed (outdated)

I'm closing this in favor of the media_library integration

grantkruger’s picture

Thanks for the patch reroll, @rudins.

Dennis Cohn’s picture

There are still many of us who still haven't migrated to core media.

Indeed... I want, but there isn't an upgrade path from file entity to media core, or is there a nice way to do this??

Anyway, the patch is failing on 1.4 :(

sandzel’s picture

Adjusted patch for 1.4

Dennis Cohn’s picture

Anyone who can make a re-roll for 1.5?

reszli’s picture

here's the same patch against latest dev (which also applies to 1.5)

samuelpodina’s picture

Version: 8.x-1.x-dev » 2.x-dev
FileSize
25.45 KB

here's the same patch against latest 2.x dev (which also applies to 2 for D10)