Hey, nice module, thanks! I've just added some tweaks to eliminate a DB query on every page load and to allow the choice of unpealed and pealed image sizes, as well as fix an issue where the containing div has marginRight or marginLeft which messed up the alignment of the pageear. I've attached the patched module.
The only trouble is, with the original swf, variable flag and peel sizes aren't supported (but you still get the benefit of the saved query and margin fix). I updated the swf to the latest from webpicasso, but it only seems to work reliably in Firefox, not IE... oh well. The example on the webpicasso website seems fine in IE, so it's probably a matter of tweaking the parameters..
You'll find the files that use to original swf saved as _orig, so you can take your pick.
Enjoy.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | pageear.zip | 440.25 KB | joehudson |
| pageear.zip | 310.06 KB | joehudson |
Comments
Comment #1
manfer commentedThat sounds a good idea to have a variable tracking if there is any pageear enable but inspecting your code I think there are errors. I'm going to test.
I'm going to review the module code and I'll try to not only avoid one of the queries when there are no pageears enable, just avoid both of the queries. I'm thinking of changing all test code for enabled pageears from hook_footer to hook_init. That would even correct one more thing, javascripts where being loaded even on nodes with no pageear (one or more pageears are enabled but not visible on that specific node).
All the rest about using the module interface with webpicasso pageear would need more work than the ones in your changes. There are more parameters in this module dont needed on webpicasso pageear. But this module does not use webpicasso pageear since version 2.
The fix on marginRight or marginLeft must be some fix on your work on the module. There is not marginRight or marginLeft on 6.x.2.0-rc3, the flash movies are absolute positioned on top left corner (left:0px and top:0px) or top right corner (right:0px and top:0px) as needed.
I would look to provide the posibility for different size pageears. That needs flash work.
Comment #2
manfer commentedTested your code on firefox 3.5.2 Mac OS X and it is not working at all. I don't know exactly where you tested but your code has errors.
And not only pageear is not being displayed if you try to change configuration of a pageear you get a query error.
I dont know exactly what you are trying because I dont understand the pageear parent element id you are including on configuration form. Pageear is an ad for left or right top corner of page not for any other element on page. The configuration of any pageear is saved on database. If you include new parameters database must be updated too and of course all queries.
The code as you provide it has a lot of errors, sorry.
As I mention maybe it is a good idea to track number of enabled pageears with a global variable. I would look on it.
Thanks for your suggestion.
Comment #3
joehudson commentedthanks for your feedback. It could be I uploaded pre-working version. if so I'm sorry about that. took me the best part of a day hacking about with it (and looking for a way to decompile the swf (as the fla files were unreadable) and failing ), I was rather tired when I uploaded it.. But now after further tweaking I've got it working in IE and Firefox (only now it's slightly sketchy on FF, the 2nd time you activate it on the same page.. preloading the image would be handy too)
Yes I have included an updated .install file. The config variables of the pageears are converted to the other format for the webpicasso script. I would say your script is generally better (but would be nice if the large peel wasn't so curved, because it reduces the space to display the image significantly), only it doesn't support variable size flag and peels, which is what I need.
I get no errors when administering pageears with these updates.
Re marginRight/Left, the pageear is inserted where ever the closure is printed in your themes page file. If you want the pageear to appear on your main content div (which looks pretty cool) instead of the actual window corner then you need to account for things like the margin value of the parent div. That's what I've done. This also opens up the possibility of having pageeared blocks... maybe a bit OTT.
So, here's the updated files which I'm using now on this site:
http://www.healthylovingrelationships.com
cheers
Comment #4
manfer commentedOn your site now is working.
I supposed the parent id was for something like that. Maybe I could look to include that posibility for people that would like to include pageear on an inner div. It has the problem of mouseout when you leave pageear to right (or to the left if it were a pageear on left) but maybe some more people like it.
Now that seems to work I could try it and inspect the code for that global variable of number of enabled pageears that on previous version you provided looked not perfect. There were functions where the variable should be updated and it wasn't.
I would look to provide those features on future versions.
As for the no so curved peel I could look to do it less curved but I think a little curve is more realistic and graphic file can always be adjusted for the pageear curve.
The fla files I provided should be readable. Which program you used to try to open them? Maybe I have to save them a different way.
The flash movies included with 6.x-2 versions has preload of media files. I can't tell anything about if it is done or not on the flash movies you are using in that version.
Comment #5
manfer commentedFor some reason the update6300 was not being done on my test site, nor on install nor on update.php. Now I have just selected it manually on update.php and the query errors dissapear. The errors were because the new columns didnt exist without doing that update6300. Maybe your previous uploaded version was correct too and this was only what was failing to me.
I still dont know why that update6300 is not triggered. I uninstall the module and install it again and I always have to trigger that update6300 manually on update.php. I have to select it manually or it isnt done nor on install nor on update.php. Shouldnt it be selected automatically on an update.php if needed? Shouldnt it be triggered just on install?
As I supposed that global variable tracking the number of enabled pageears is not correctly implemented and there are functions where it should be updated and it isn't. It is easy to foul the module about the correct number on that variable. For example, now I have two configured pageears, both disabled, so that variable should be 0, but it's value is 5.
When you disable the module (just disable not uninstall) all enabled pageears are disabled (not deleted just disabled) so that variable must be reseted to 0 on hook_disable.
Pageears can be enable/disable from configuration form. That variable must be updated on configuration form submit.
Those are the two errors. On your code that variable is only updated on enable, disable and add submit.
Thanks for all suggestion and I'll implement them for future version if I'm able to.
Comment #6
manfer commentedI have tested your code and I'm not able to make it work.
The sizes can be changed. That works fine. But the parentId feature is not working.
I looked at code and, if I understand how it is working, you compute a left or right margin for the pageear based on the parent div. But there is nothing about the top margin. Incredibly anyway it is working in your site though there are around 20px over the pageear.
Anyway I'm trying in a test site and I'm not able to put the pageear on any parent div. The horizontal margins (right or left are not computed correctly). About top it is always on top as expected.
I actually cant understand how it is working on your site. Maybe your theme has a layout that makes it work but I think it is not a code that works on a lot of cases.
Maybe I'm wrong and I don't know how to use it.
Comment #7
manfer commented@joehudson
After analizing your code I think this can't be exactly the code you are using on your site. Or maybe there is something I'm missing.
In your site the pageear div layers are inside the parent container where you want to position your pageear. I can't see that in the code you provided at all. The pageear div layers should reside at the very bottom of the body and outside any other divs. So, or the code you are using on your site is different, or any other code on your site is moving pageear div layers inside that parent container you wish it to be. Anyway this is not really a problem it is easy to move pageear divs to a parent div container and would not be any problem to include this on the module.
But that is not the only problem. There is another reason why your code is working on your site. That parent container where you put the pageear is inside another div that has its position property set to "relative" and that is why it is working on your site. Your theme layout permits the absolute position of the pageear being relative to that layer. But it is not a common situation. I think your code wont work in most themes.
I have worked on flash and now I have a new version with custom size pageear feature but about the custom position of the pageear is something still not sure how to deal with it because it seems highly theme dependent. I have to think more about it.
Comment #8
manfer commentedI have added the performance improvement in 2.0-RC5 versions of module.
I'm working on the custom size feature. I will release it as a development version and would be added in 2.1 versions.
As I said for the custom reposition is something I have to think how it can be done.
Comment #9
joehudson commentedHi!
wow, only just seen all your replies. it's great that you're implementing the the variable size feature.
re the curve of a peeled page, actually when I peel the page of a book from the corner the peeled page is pretty straight in the middle. it's just at the edges it's curved. anyway, it's a small point and I suppose you can just make the peeled area bigger to accomodate a curve.
re the parent div feature. the way the module was written, the pageear div is simply inserted in the closure of the page.tpl.php file. so if you put print $closure inside some inner html divs (as I have) that's where it will be put. If you look at the pageear.js you'll see how I compute the margins (yes I missed out top, my bad). The parentid is simply the id of the div that wraps the $closure. Make sense?
Yeah I had the same issue with the 6300 update. I'm sure there's a simple explaination, but it was a case of 'I've spent far too long hacking on this already, just get it working and move on'. The code I uploaded was definitely the code that's running on my site.
Well done for spotting those errors with the active pageear variable!
I tried opening the fla files in ConTEXT editor and they look like they were encoded. Lots of strange ASCII.
cheers.
Comment #10
manfer commentedI have just commited a dev version with custom sizes. Within a few hours (I don't know exactly how often the packaging system packages dev releases) it would be available.
I didn't corrected the curve on peel. Maybe I could try to smooth then a little next time. By now I just implemented the custom sizes feature.
So that dev version includes the performance (included on 6.x-2.0 version too) and variable peel sizes (by now only on the dev version).
I would continue trying with the parent id feature but as I said is something I have not clear. I would analize what you say in this post but I think the problem is the one I mentioned before, that feature is theme dependent and in your theme works because the inner div in which you insert the pageear has its css position property set to "relative".
About the fla files, they are flash files and can't be opened in a text editor. But they are not encoded, you only need a flash editor to open them.
Thanks for all your suggestions.
Comment #11
manfer commentedI added two new changes to the development version.
Added peel color style, you can decide to use a flat or gradient color on peel.
Added peel styles. By now I included two styles, the one curved just available in previous versions and now another smooth style with more space to display the image.
I decided to include that new feature instead of modifying the available style because maybe other users like the style of version 2.x. This way users that have designed there images to fit in version 2.x peel style can still use that style and users that prefer the new style can use that new one. How I coded it now opens the posibility to include more styles in future versions very easily too.
The packaging system for dev versions runs every 12 hours so it is not available yet, but would be in a few hours.
With that version and if you like the new style you would only need to include a minor change to the javascript file that writes pageear div layers including there the margins to place it correctly on your site (I would not change the module to include divParent, unless you plan to use different pageears on your site in different nodes, you really dont need it). If I understood how it is working on your site, in your theme the closure is printed (I bet with theme_closure) in an inner div which has its position css property set to relative and the absolute position of the pageear will then be relative to that div. By providing the margins (you dont even need to write the code to compute them) in the javascript pageturn.js file manually you would end with a pageear located correctly in that inner div in your site.
I will inform when the new development version is packaged and available in the module home page to be downloaded.
Comment #12
manfer commentedThe new development version is available for download on the homepage. It includes the new three features: custom peel sizes, peel color style, peel styles.
Comment #13
joehudson commentedthanks manfer, good work!
Comment #14
manfer commentedI'm going to mark this as fixed cause both things are just included on dev version.
I won't forget about the custom pageear position but by now I don't know how to do it in a way valid for all themes. If someone has any idea about a viable solution please open another issue and we all could debate it.
Thanks for all your suggestions.