Opened 10 years ago

Closed 10 years ago

#9073 closed defect (fixed)

dojox.image.lightbox window resize inconsistent

Reported by: Aleksey Rechinskiy Owned by: dante
Priority: high Milestone: 1.3.1
Component: Dojox Version: 1.3.0
Keywords: Cc:
Blocked By: Blocking:

Description

This bug report is about some issues in changeset [17205] to dojox.image.lightbox. Component works much better, than the original version, shipped with dojo1.3, does, but still has some serious bugs, preventing its real-world usage.

All issues has been found in FF3.0.8

1) onclick on underlay doesn't work when "large than viewport" image opened (while it does with other pictures).

2) set browser window to say 1/2 of total screen height and width, then maximize the window. Open "large than viewport" image, then restore window to small size. You will not be able to close lighbox at all: page layout is damaged, scrollbars aren't workable.

3) maximize browser window, reload page. Open "large than viewport" image, then close it. Restore the browser window to small size. Notice a horizontal scrollbar appears. There is no reason it should be there, but it does.

Change History (13)

comment:1 Changed 10 years ago by Adam Peller

Milestone: 1.3.1tbd
Owner: changed from Adam Peller to cb1kenobi

comment:2 Changed 10 years ago by dante

Milestone: tbd1.4
Owner: changed from cb1kenobi to dante
severity: majornormal
Status: newassigned
Summary: dojox.image.lightbox still have problems in cs[17205]dojox.image.lightbox window resize inconsistent

the behavior in 1) is expecpted, as per the modal="true" option passed to that single element. I know _what_ is causing 2), I even put a FIXME: in the code where _size had to be overridden. That hook should allow me to calculate scaleToFit (which is the same as _size) on each window resize if necessary.

comment:3 Changed 10 years ago by dante

3) is baffling. I didn't notice before, but looking around there is nothing in firbuge indicating anything is taking up space, nor have any elements been styled. It appears to be approximately the right-edge of the lightbox after closing.

comment:4 Changed 10 years ago by dante

Resolution: fixed
Status: assignedclosed

1 and 2 should be fixed in [17213], I've opened #9080 to trac issue 3 -- please test this new iteration, and comment on #9080 or filing new tickets for anything further.

thanks for you help in reporting these, having reproducible steps makes fixing loads easier.

comment:5 Changed 10 years ago by Aleksey Rechinskiy

1 and 2 have been fixed, confirm.

Although, it's not an error, but why lightbox footer is so big now? It's almost 3 times bigger, than requires a small string it displays . In previous revisions it looked just perfect and scaled with height of text it displays. Could you please make it variable-height? It would greatly improve it visual appearance.

thanks for you help in reporting these, having reproducible steps makes fixing loads easier.

Let me thank you for such a quick fix. This is great, when bugs are fixed so fast and makes me and everyone, who uses Dojo believe, the Dojo is truly right choice.

comment:6 Changed 10 years ago by Aleksey Rechinskiy

and by the way, I've just tested in IE6 and it looks like the footer is variable height, but strange enough. For the same picture it can vary from time to time depending on pictures that were opened before. This behavior is similar to older versions, but I thought it was fixed since 1.3.

Should I open a new ticket for that?

comment:7 Changed 10 years ago by dante

Resolution: fixed
Status: closedreopened

yes, I might just back out the #7272 fix for now and go back to the old behavior. It is reporting odd sizes in different browsers (as you've noticed) and it changes from image to image, as well as reports a "correct" size at the time of calculation, but when the image container goes to resize more space is provided, and the calculations are faulty (eg: text only takes one line now)

what do you think? I've tried a number of things to normalize the size calculation, but nothing is working, whereas the forced single-line pattern was working well. The issue is I need to know the vertical space of the title text in order to know if it fits in the viewport before doing the animation. I suppose I could animate the width first always, calculate the titleheight, then animate the height, but maybe just reverting to a fixed "30px" height and exposing that number as a property would be the easiest/safest solution.

comment:8 Changed 10 years ago by Aleksey Rechinskiy

reverting to a fixed "30px" height and exposing that number as a property would be the easiest/safest solution.

It is easiest solution, that is true, but moreover, I think, - the worst.

animate the width first always, calculate the titleheight, then animate the height

is much better. If it's best - I'm not sure yet, let me ask you a question: after the inital resize is completed and Lightbox with a picture is shown, do you a have a reliable algorithm to calculate a height the text requires? If so, you may leave lightbox resizing as is, but add to its end an animation to adjust lightbox footer size to a correct value. It is not best solution, but much better then exposing a property and slightly better, than resizing lightbox width and then height, because the picture is shown to the user as fast as possible.

I don't know how to calculate a height that some text require to display... The dirty way I can imagine, is to create visibility:hidden floated DIV with fixed width, height:auto and innerHTML set to the text. It will automatically adjust its height to fit all the text. You will only need to get its height after that, add it to the total Lightbox height and then do the resize animation. Its dirty way, I know, but it looks not worser, than creating invisible image to preload a picture or to obtain its size.

By the way. I want to suggest to suppose, that picture title can be any length. And if it require too much height to display (like more than 1/5 of the screen height - this value should be exported), Lightbox should draw a vertical scrollbar to it. Specifically my project would very appreciate that functionality.

Thank you for your work. Wish you a good luck.

comment:9 Changed 10 years ago by dante

@Arech - please checkout [17216] and see if that doesn't resolve the titleText issue. I even added a "custom html" thing set to a fixed 75px that should allow enough context to accomplish your scrollbar idea. will mark this as closed once I hear back.

comment:10 Changed 10 years ago by Aleksey Rechinskiy

Yep, nice work! It looks like everything is fine now. I'll test it more, when I'll be able to integrate dojo1.3 in my project, but for now - it is OK!

Thank a lot for the Lightbox :)

comment:11 Changed 10 years ago by dante

Resolution: fixed
Status: reopenedclosed

comment:12 Changed 10 years ago by dante

Milestone: 1.41.3.1
Resolution: fixed
Status: closedreopened

backporting to branch

comment:13 Changed 10 years ago by dante

Resolution: fixed
Status: reopenedclosed

(In [17315]) fixes #9073 - backporting current Lightbox changes into 1.3.1 - they were deemed significant and breaking enough for branch

Note: See TracTickets for help on using tickets.