Hello and thanks for the distribution, I am learning a lot :)
I have an Open Atrium installation and I was using Manualcrop on images styles. I got this error https://www.drupal.org/node/2092911 that now is solved in the last dev. Also about the patches that you apply on Manualcrop
- http://drupal.org/files/issues/manualcrop-file_entity-settings-1665130-1... //it is commited in last dev
- http://drupal.org/files/issues/manualcrop-two-crop-tools-2237835.patch //needed as @dsnopek said in #3
- http://drupal.org/files/issues/manualcrop-make-file-2248587.patch //it is commited in last dev
This is also also needed
- https://www.drupal.org/files/issues/manual_crop-fix-for-array-flip-22924... //needed it is commited in last dev
Thanks again :)
| Comment | File | Size | Author |
|---|---|---|---|
| #24 | panopoly_images-manualcrop-update-2401473-24.patch | 1.75 KB | dsnopek |
Comments
Comment #1
dsnopekHere is a patch that make the changes described in the issue summary. But I haven't had a chance to actually do any manual testing of the new manualcrop, which will be necessary before this can be commited.
Comment #2
candelas commented@dsnopek I found a bug in manualcrop-two-crop-tools-2237835.patch that makes that when you add a new image the manualcrop styles are lost. I reported here. Thanks for your work.
Comment #3
dsnopekHmm. That patch was added to support #2255095: Add "Crop" tool to Spotlight widget which was reverted. So, we could actually remove that patch until that issue is merged! Here's a new patch that does that...
Comment #4
candelas commented@dsnopek could be that in that patch you made an array and manual crop has not that option when creating a new image crop? Thanks to be so fast :)
Comment #5
dsnopekI'm really not sure what could be causing the problem, I haven't had any time to look into it. But since we reverted the functionality that actually needs that patch, we may as well worry about it later when adding back that functionality. :-)
Comment #6
candelas commentedI just saw that they change last dev. The one I checked and I am using is 7.x-1.4+121-dev
I check the new dev and the only feature added is this
Add configure link on modules page
what I think doesn't interference.
Comment #7
dsnopekComment #8
dsnopekTrying patch on Travis-CI: https://travis-ci.org/panopoly/panopoly/builds/53462750
Comment #9
candelas commentedThe only Manual Crop blocker #1828178: Media 2.x UI suggestion in the Media file selector widget to be have next release #2306801: Upcoming 1.5 release
Comment #10
candelas commented@dsnopek The patch that I mention in issue summary
- https://www.drupal.org/files/issues/manual_crop-fix-for-array-flip-22924... //needed
has been commited. https://www.drupal.org/node/2292491
The Manual Crop mantainer has said in #2306801-7: Upcoming 1.5 release that at the end of next week he will be willing to release 1.3. Also he says that if there is any tested patch that we, users, want to be committed, that we say in that issue.
Since I know little, maybe it can be interesting for you to take a look on their patches that need review, so in the future you don't need to patch it.
Comment #11
dsnopekIt'd be very welcome if a new stable version of manualcrop was released! All but one of our patches has been merged upstream already and the remaining one is kind of unfinished anyway. So, I don't think there's any patches we really need to push for to be included! If you see that they make a new manualcrop release, please let us know on this issue. Thanks!
Comment #12
candelas commentedMainteiner said:
Now that #1828178: Media 2.x UI suggestion in the Media file selector widget has been fixed I'll release a new version next saturday! So please give
the lastest dev version a test drive to prevent major issues in the next
release
Comment #13
dsnopek@candelas: Thanks for the update! Here's a quick re-roll of the patch with the now latest manualcrop -dev so that I can try this on Travis-CI. Then I'm going to do some manual testing.
EDIT: Here is the Travis-CI build: https://travis-ci.org/panopoly/panopoly/builds/57945062
Comment #14
dsnopekWorks great in my manual testing! Assuming Travis-CI is ok with it, I think this is committable. :-) I guess then it'll be a question of whether we commit with -dev or wait for the release next Saturday.
Comment #15
dsnopek@candelas: Can you do some manual testing with the latest manualcrop -dev in Panopoly too to see if you can find any issues that I didn't find? Thanks!
Comment #16
candelas commented@dsnopek I will try during the weekend or begining of next week. Now I am learning about features because I have made many changes to Open Atrium and I am looking for a method to keep my changes when I update version.
I would wait until the stable ManualCrop release is done, wouldn't I? Like that you will not have to update later :)
Comment #17
candelas commentedI change the subject and if I report any problem, I will make it related :)
Comment #18
candelas commentedComment #19
jiv_e commentedCan I help with this? I have problems that are fixed in the latest stable manualcrop (1.5). By the way what is the recommended way to update a single module in panopoly for my own project?
Comment #20
dsnopekYes! Doing manual testing with Panopoly and the latest Manualcrop, making sure that everything works as it should, and reporting back here would help. Even more helpful (although, a lot harder to do) would be creating some Behat tests for the Manualcrop functionality as exposed in Panopoly so we can merge in updates quicker (since we won't need to manually test as much).
If we're just talking about a single site (as opposed to a child distribution) and this is just for testing purposes, here's what I'd recommend:
drush updb -yor update.php)But once Panopoly does this update, be sure to remove your extra copy from sites/all/modules - otherwise you'll miss out on future updates from Panopoly.
Thanks!
Comment #21
jiv_e commentedGreat! Thanks for the answer! Panopoly_test has crop mentioned only in two places. Current tests check that the crop button is available on media media browser and when adding a spotlight.
What kind of tests for cropping would be most critical in your opinion? I could try to look into that if I get some free time in the future.
Comment #22
dsnopekIdeally, it would actually use Manualcrop to crop an image and then verify that an image with the correct dimensions was generated. I think that'd be enough! However, it will be a bit challenging because so much of this will need to be done with Javascript.
Comment #23
dsnopekHere is a new version of this patch which updates to manualcrop 1.5, and includes #2499647: Update to jquery.imgareaselect v1.0.0-rc1 (so we can download from GitHub) to try and fix #2499595: Build failing because host of jquery.imgareaselect (odyniec.net) has gone offline
I've done a little manual testing, I'm going to do some more in a bit. I think I'll probably end up making a new issue for writing the Behat tests for manualcrop, because I want to get this merged soon!
Comment #24
dsnopekAlright, that patch was short lived. :-) The maintainer of manualcrop made a commit to pull the 0.9.11-rc1 release from GitHub:
http://cgit.drupalcode.org/manualcrop/commit/?id=d6c449d68826b034492460b...
So, here's a newer patch which updates to that version!
EDIT: Here is a Travis build of this patch: https://travis-ci.org/panopoly/panopoly/builds/65247524
Comment #25
dsnopekHere is a follow-up issue to write the Behat tests: #2499699: Write Behat tests for ManualCrop
My manual testing is looking great! If the automated tests pass on Travis, I'm going to commit this.
I'd feel better if we had more testing/review, but since this is breaking our build and essentially preventing us from being able to release, I'm going to push it through.
Comment #27
dsnopekCommitted! Thanks to everyone who helped to push this issue forward :-)