Hi Rus
I have yet to install your amendments, and have only had a brief opportunity to compare your code.
Something I discovered since, however, is that my event image code in tpl_events_calendar_default (lines 172-180) seems to work if zen_image is replaced by zen_image_OLD.
In an effort to understand why zen_image_OLD works, I created the following thread:
http://www.zen-cart.com/showthread.p...-zen_image_OLD
While I await responses, I am wondering whether all the fine work you did with image manipulation could be replaced by simply using zen_image_OLD.
Cheers
ps No malice intended.
Well deserved praise my friend!!!
**Coming out of lurk mode**
Though I haven't reviewed the latest versions of this module (when I was looking at this I opted for a custom solution that included event invites and member only capabilities -- so I ended up not using this mod down for that project) basedon the descriptionof the changes alone, it doesn't appear that Rus' changes would interfere:
2) When you go to review an added event the file appears to be written regardless of which error flag is set. The only way to easily (without chasing down the code) avoid writing the image would be to blank the _POST image variable and reload the script. To avoid that I put in a workaround that deletes the file at the end of the script if an error was flagged.
3) I put in a define (EVENT_FILE_MAX_WIDTH) at the beginning of events_manager.php (it is currently set to 600) that is used to check against the width of the uploaded file, if it is greater then the image is reduced to 600 px wide...height is proportionate.
4) In my scenario I could think of no good reason not to overwrite an existing file. Perhaps it would have been better to use a popup and ask to overwrite the image...instead I just check for the old, remove it and save the new. This means any pre-existing event that used the old image will now use the new image.
My Site - Zen Cart & WordPress integration specialist
I don't answer support questions via PM. Post add-on support questions in the support thread. The question & the answer will benefit others with similar issues.
Hi Rus
Your amendments installed fine, except for the following issues:
1) Using the Browse button to upload images allows me to upload and insert any file type, not just jpe*g, png, gif, despite the error messages. Solution may be to copy and edit .htaccess from /images/ to /images/events_images/, but doing so may negate some of your code.admin/events_manager.php
admin/includes/events_manager_drop_dns.php
Generally, however, I prefer to adjust images offline and upload the adjusted images as a separate process, and then only use the browse button to select images from events_images folder.
I am thinking of changing the layout of the image table in event manager so that the name of the selected image and the "delete image" checkbox are both situated directly underneath the browse box. This would provide for a less cramped layout.
2) While I can select a Manufacturer, I cannot select any Category/Product, nor overwrite any selected manufacterer with Category/Product.
Cheers
From what I can see adding cloning the images/.htaccess file shouldn't hurt anything (and is something you SHOULD include anyway for security purposes) It shouldn't interfere with the new code since based on your description Rus' code does do a check for other filetypes, but isn't preventing the actual upload.. If this is the case, then the images/.htaccess shouldn't interfere..
1) I noticed that any file type could be uploaded when attaching images. If a client doesn't know what they are doing and they upload certain file types that could be a security risk. So I check for jpg, jpeg, gif, and png and if the file is not any of these it shows an error.
My Site - Zen Cart & WordPress integration specialist
I don't answer support questions via PM. Post add-on support questions in the support thread. The question & the answer will benefit others with similar issues.
First...no offense taken. I am in no way going to try and force code down the throat of someone who has put forth the effort that you have on this project.
Oh boy...I wrote a lengthy reply...took about 45 minutes, I went ahead and hit the spell check, it then asked to install ispell which closed the browser, when it came back up the message was gone. Here's the short version (aww man and that other reply was so witty and interesting):
I have a friend who is an IT manager and he taunts me about spending too much time on resource management. In the world of unlimited storage and bandwidth this idea may be a little "old school". Originally it was done to save the store owner from himself, he uploaded a 3k px wide photo and the results freaked him out a little. Now he has a list of how wide images in different places should be (manufacturer logos, products, etc.) and knows to just re-upload another photo, so me adding the resizing code is just a feature for him now (plus it only works for the events' photos). On the tech side of it the resizing of the image to whatever its max viewable (delivered) width will be for the site will save on storage and bandwidth. I looked at the 2 zen image functions and they resize the image by changing the width and height fields on the html IMG tag. What this does (unless the web server has been enhanced to resize images before delivery) is send the whole disk image of the photo to the client side browser, basically full size, then the browser resizes it before viewing based on the html tag or css code. So for every hit of that photo the server sends out the full size image (1st time, then its cached on client side). If the store owner/admin is not web savvy, they might upload a very large photo and if your new code on the service sevice changes it to fit the container they may never know the true width/height (if they pay attention to the preview before the update this shouldn't happen.) Anyway, I know we are talking about nickels and simes here, but in my opinion (and after I become more familiar with Zen Carts I may try to tackle this...time, time, time, blah.) there should be settings for all the different types of images (manufacturer, inventory, category, etc.) for a max width/height, and then a function that would resize & save at every admin upload. Just talking efficiency in small quantities...over time it could add up. I have 2 virtual servers and my resources are limited on both, but where it really helps is on the client side because they're not being fed images that are larger than need be. Even on those servers that have code to resize before sending (I didn't see this in Zen Carts), your talking extra cpu cycles and disk I/O every time someone on the net is seeing that photo (for the 1st time).
<grin> That's the short version.
Thanks again for your work on this project,
Rus
Hi Rus
Well I for one am very glad you came along and took the time to positively respond. It's always good to have another pair of eyes and someone else to talk with, however long the conversations may last.
I have adopted a good number of your changes and intend to submit an update to plugins as soon as I add some words about image sizes etc to the readme and rejig the event_manager image box - hopefully all by this weekend.
At last I am feeling good about the mod.
Cheers
1) I remember running into trouble and trying to figure out why I couldn't get the event_image variables to reset. The file is being deleted when it is not an image, but the url was still being written which then shows a broken image box. I see now(I found the code) why it is happening, but the solution wound up being really simple...
In the code I added to admin/events_manager.php around line 92 I have:
...just 1 line has to be added, it should look like this:Code:if (!in_array($fileext, $validext)) { $messageStack->add('Uploaded file ' . $working_file->filename . ' is not a jpg, jpeg, png, or gif.', 'error'); $event_error = true; $success = false; $file_to_delete = $working_file->destination . $working_file->filename; } else {
Code:if (!in_array($fileext, $validext)) { $messageStack->add('Uploaded file ' . $working_file->filename . ' is not a jpg, jpeg, png, or gif.', 'error'); $event_error = true; $success = false; $file_to_delete = $working_file->destination . $working_file->filename; $_POST['no_image'] = 'on'; //**rus: new line, simulate removal of image url } else {
2) Odd, on my version it is working. In the file admin/includes/events_manager_drop_dns.php there are probably 3 places to look:
around line 31...
...the option's value has to be "-1", it triggers an event in events_manager.php.Code:$select_box .= '<option value="-1">No Manufacturer</option>';
Also make sure the following javascript additions are there, that's what zaps the manufacturer option for you.
around line 79...
...and around line 113...Code://** rus: start $dum_oc = "document.getElementById('manufacturers_id_sel').value = ''; submit();"; $info_box_contents[] = array('form' => '', 'align' => 'left', 'text' => zen_draw_pull_down_menu('cPath', zen_get_category_tree(), '', 'onChange="' . $dum_oc . '"')); //** rus: replaced: $info_box_contents[] = array('form' => '', 'align' => 'left', 'text' => zen_draw_pull_down_menu('cPath', zen_get_category_tree(), '', 'onChange="submit();"')); //** rus: stop
The javascript will probably have to be added to the upcoming products select box also...my client wasn't using the feature, but I could do that if needed, just let me know.Code://** rus: start $info_box_contents[] = array('form' => '', 'align' => 'left', 'text' => zen_draw_pull_down_menu('products_id', $products_array, '','onChange="' . $dum_oc . '"')); //** rus: replaced: $info_box_contents[] = array('form' => '', 'align' => 'left', 'text' => zen_draw_pull_down_menu('products_id', $products_array, '','onChange="submit();"')); //** rus: stop
-----------------------
I'm hoping I remarked all of my changes, I'm going to feel really bad if I didn't. If you want I can look at the 2 files you changed on your end and find out if I missed something.
Also, when I was testing I noticed another bug (and some bad grammar) that I introduced, nothing major, just an annoyance...when there is no file being uploaded an error is triggered which shows a message (...file may be too large). So around line 162 of admin/events_manager.php where it says...
...change it to...Code:$messageStack->add('There was a error uploading your image file (may be too large)', 'error');
One last thing I saw was another little annoyance. I didn't round the height to an integer when resizing. It didn't affect the functionality, it just showed decimal places when printing the resizing message. So around line 105 of admin/events_manager.php...Code:if ($_FILES['event_image']['error']!=UPLOAD_ERR_NO_FILE) $messageStack->add('There was an error uploading your image file (may be too large)', 'error');
...change to...Code:$nhei = $ehei * ($nwid / $ewid);
Code:$nhei = intval($ehei * ($nwid / $ewid));
Let me know if I can help in any way.
Thanks again,
Rus
I agree with your reasoning behind the image code and IMHO something like this SHOULD be incorporated into this module.. The whole purpose of the module Image Handler is to automate the re-sizing and optimization of the product and category images. This helps shopowners do something automatically they may not remember to do on their own.
IMHO, something similar SHOULD be a part of this module for ALL the reasons you stated.. Even with instructions in the readme, novice DIY shopowners are not always going to follow these instructions and the module should include some controls to help shopowners engage in best practices with regards to image management. Adding some "Image Handler" like image size/management features including admin controls to set the max image height and width AND using the Zen Cart option to re-size the images proportionally would do the trick.. This would enforce better image management practices, and give shopowners some measure of control over the image sizes..
My Site - Zen Cart & WordPress integration specialist
I don't answer support questions via PM. Post add-on support questions in the support thread. The question & the answer will benefit others with similar issues.
I recall seeing one instance of -1, but was not aware there were three places to look. Will have to revisit.the option's value has to be "-1", it triggers an event in events_manager.php.
Yes thanks. Upcoming products would play nicely with upcoming events.The javascript will probably have to be added to the upcoming products select box also...my client wasn't using the feature, but I could do that if needed, just let me know.
Cheers
ps. To make code changes easier to spot in the forum, may I suggest changing the color of relevent section of code to say red/blue = old/new.
Must fly now (literally)