Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.The initialization of cropper currently looks like this:
$cropWrapperSummary.bind('click', function (e) {
var $element = $(this).parents(cropWrapperSelector);
setTimeout(function () {
Drupal.imageWidgetCrop.initializeCropperOnChildren($element);
}, 10);
return true;
});
$verticalTabsMenuItem.click(function () {
var tabId = $(this).find('a').attr('href');
var $cropper = $(tabId).find(cropperSelector);
Drupal.imageWidgetCrop.initializeCropper($cropper, $cropper.data('ratio'));
});There are several issues with this:
- There is no event to bind on when a tab or details element is being shown or hidden.
- Since all events we currently can bind on fire before the cropping area is being displayed, we need to use the setTimeout() workaround to let it fire later on. We can only initialize Cropper on visible elements, since it need to calculate the dimensions.
- We bind on two events where it would totally be possible with one.
- Vertical tabs behave very weird responsively. They don't get initalized if the browser is too small and therefore the click handler doesn't work. #2625848: Fix responsive wrapping for mobile clients is postponed because of this.
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | 2631710-20.patch | 635 bytes | sanja_m |
| |||
Comments
Comment #2
Lukas von BlarerComment #3
miro_dietikerI guess this is a bit related, there are some references about unneeded attributes and ratio determination and scaling:
#2641970: Cropping coordinate determination
Comment #4
Lukas von BlarerNot that much, actually. This is mainly related to flaws of vertical tabs in core.
Comment #5
miro_dietikerI fully agree about that core origin.
So can you point me at the core issue that picks up the ball to add the missing events?
If not, please open an issue. We should promote the issue on our side and help resolve this.
Also if code contains workarounds, the code should contain a reference to the issue with a cleanup @todo.
Comment #6
woprrr CreditAttribution: woprrr as a volunteer commentedWe have an issue in d.o or we need to create this issue ? I can talk this with fgm to view. I not sure to understand all problems in this issue.
Comment #7
miro_dietikerI assigned it to someone of our team to investigate it.
I have the same problem as you do. It's a workaround without any explanation.
They first need to check the d.o issue queue to see if there is an issue, or create one and describe the problem.
Comment #8
sanja_m CreditAttribution: sanja_m at MD Systems GmbH commentedAssigning to me. I am checking the possibility of resolving these issues only with JS, without involving any other vertical tabs core issues.
Comment #9
sanja_m CreditAttribution: sanja_m at MD Systems GmbH commentedCreated the patch that removes setTimeout() workaround and fixes cropper initialization for smaller devices.
Comment #10
woprrr CreditAttribution: woprrr as a volunteer commentedAfter the merge of issue "https://www.drupal.org/node/2631712" i can not seem to apply you patch, for not make stupid error I can let you do the update? thanks a lot @sanja_mComment #11
woprrr CreditAttribution: woprrr as a volunteer commentedNow it's ok i ve re-create patch and apply is now ok for that. But i ve a new comportement now, when i upload / edit an image i open "crop image" tab nothing fire, when i click an vertical tabs cropper fire initilize and all is okay. My problem is for this "step" or you do not really know what to do after having just unfold the item. Should we Grisse you the picture and tell the user to choose a format on the left? What are your views on the matter?
I add a screenshot to illustrate my think.
Just after open "crop image" tab :
Now i click on 16:9 tab and cropper is initialize (cool :))
Comment #12
sanja_m CreditAttribution: sanja_m at MD Systems GmbH commentedI will test it again and try to figure out what is the problem for that step when "crop image" tab opens, and try to fix that.
Comment #13
woprrr CreditAttribution: woprrr as a volunteer commentedSuper !! thanks :) I trust you entirely on that subject.
Comment #14
miro_dietikerJust firing the initialisation also for the default tab should be easy.
However i still say that the vertical tabs from core should offer an event that is fired where we can subscribe. That event would also need to be fired for the default tab initially.
While i appreciate a better solution without setTimeout in IWC without a core dependency, it's still critical that this lacking thing of core is escalated and need to be addressed.
Comment #15
sanja_m CreditAttribution: sanja_m at MD Systems GmbH commentedFixed bugs for chrome. Also created issue for missing events in vertical tabs: https://www.drupal.org/node/2653570.
Comment #16
woprrr CreditAttribution: woprrr as a volunteer commentedIt's Ok for me :) Great job thanks.
Comment #18
woprrr CreditAttribution: woprrr as a volunteer commentedComment #19
woprrr CreditAttribution: woprrr as a volunteer commentedERF !!! I ve found an error when you have few crop type in same field @see
Comment #20
sanja_m CreditAttribution: sanja_m at MD Systems GmbH commentedFixed for multiple crop types too.
Comment #22
woprrr CreditAttribution: woprrr as a volunteer commented@sanja_m Great job :) it's okay now, i ve tested it in all case for me it's OK.
Comment #23
woprrr CreditAttribution: woprrr as a volunteer commented@sanja_m i think we need your help in this issue few new js comportements (strange). #2654210