brunoais wrote:@Marc ... Personally, I don't like that code. There are parts that are too repeated. For example, the $() is executed excessively.
There's no need to execute that function that many times. You could simplify the browser's work with very simple changes to that code. IMO, if you reduce the use of the $() to the amount you really need, then it's enugh.
@Arty That is not entirely true. You may be surprised with the results with some rendering engines if you don't explicitly specify that you want the image width to be a certain value.
Now, the ultimate question:
Why are you doing that with js when CSS can do the same job but in a much more efficient way?
That code is over 2 years old so I'm guessing there is no problem with improving it. My main goal was to get people to talk about it again. A simple image resize should be added to phpBB 3.1.
Your CSS example is IMHO way to slow. If you click on it by accident then your second click will not be recognized. That's the downside with CSS properties.
A combination of CSS & JS should be the goal here. Also I don't like picking a random number like 300px as maximum width. That's why I used the width of div.content and substracted 30px from that. That way users don't usually have to click on the picture in order to actually see what's on it.
callumacrae wrote:
You shouldn't be adding a class - the server has no reliable way of knowing whether the image is too big or not. It's also not very clean when it can be done entirely in one language.
That's why I added this:
Code: Select all
var maxWidth = $('div.content').innerWidth() - 30;
If we use CSS & JS we obviously would need to set a low maxWidth which kind of is against what I suggested. I don't think there is something like
max-width: 98%
or something though, so it wouldn't exceed the width of the parent, or did I miss something?