On window resize, titles on a "Browse themes" pages overlaps downloads number

VERIFIED FIXED in 5.0.9

Status

defect
VERIFIED FIXED
10 years ago
3 years ago

People

(Reporter: Milos, Assigned: neilio)

Tracking

unspecified
5.0.9

Details

()

Attachments

(4 attachments, 3 obsolete attachments)

Reporter

Description

10 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.11) Gecko/2009060309 Ubuntu/9.04 (jaunty) Firefox/3.0.11
Build Identifier: 

When a user browses themes on a AMO, and then resizes a browser(it must not be maximized), theme titles overlaps a downloads number. Screencast of this bug in action can be found at (http://www.mozilla-srbija.org/ext/amo-titles-resize.ogv).

Reproducible: Always

Steps to Reproduce:
1. Open https://addons.mozilla.org/en-US/firefox/browse/type:2/cat:all?sort=popular
2. Resize a window (it mustn`t be maximized)
3. Take a look at theme titles and download number text
Actual Results:  
Theme title and downloads number lines overlaps.

Expected Results:  
Theme title and downloads number lines should not overlap.

This bug was found by :mauke
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reporter

Comment 1

10 years ago
I`ve added a patch to this CSS.
Attachment #385941 - Flags: review?(fligtar)
Reporter

Updated

10 years ago
Attachment #385941 - Flags: review?(fligtar) → review?(clouserw)
Whiteboard: [patch]
Target Milestone: --- → 5.0.8
Reporter

Comment 2

10 years ago
clouserw: As per IRC, second patch doesn`t include the changes made in first one, so if you push first and than second one, problems solved in first one will be problems again.
Reporter

Comment 3

10 years ago
I`ve added another patch.
Attachment #385941 - Attachment is obsolete: true
Attachment #386800 - Flags: review?(clouserw)
Attachment #385941 - Flags: review?(clouserw)
Reporter

Comment 4

10 years ago
Posted patch Patch (obsolete) — Splinter Review
Attachment #386800 - Attachment is obsolete: true
Attachment #386801 - Flags: review?(clouserw)
Attachment #386800 - Flags: review?(clouserw)
Attachment #386801 - Flags: review?(clouserw) → review-
Posted image screenshot
Your patch does prevent overlap, but on small screens (which was the issue here) it just pushes the words out of the container.  screenshot attached
Duplicate of this bug: 501744
Neil, do you have any ideas for this?
Assignee: nobody → neilio
Reporter

Comment 8

10 years ago
Posted image Screenshot
When I add max-height: 100%;(or similiar) to my patch, then we have a problem with the thumbs description which goes in the 4th line. Then, if the next thumb in that row has it`s description with number of downloads in 3 lines, the first thumb from bottom row will stick to the right side of the thumb with 4 lines of description. So the only reasonable solution(to me at least) is to separate thumbs in rows, and then we could add float: left; property to a div that contains a whole row(4 thumbs).
Reporter

Updated

10 years ago
Target Milestone: 5.0.8 → 5.0.9
Reporter

Updated

10 years ago
Status: NEW → ASSIGNED
Assignee

Comment 9

10 years ago
I took a look at this - I can make this better, but because of space limitations it completely changes how the metadata is laid out:

http://snaps.beatnikpad.com/themes-fix-20090819-180017.png

Also note that the height of the individual theme thumb has to be fixed otherwise content shifts all over the place.

Milos' suggestion is probably the best solution on top of what's already here - separate each row into its own div and then set the height of each thumb box to min-height: 220px.
Assignee

Comment 10

10 years ago
clouserw: What did you want to do with this? We can commit the changes as is and live with the alignment issues for screens under 900px wide and look at updating the PHP for a later bug, or I can work with someone who can make the required PHP changes. I'd not a PHP hacker (at least you don't want me hacking in there) so I need someone else's help to really fix this correctly.
I'll wrap the individual lines in <ul>s, let's see if that works well for us.
Assignee: neilio → fwenzel
Neilio: Please add a patch fixing the CSS issues -- I can add the per-line <ul>s then.
Assignee: fwenzel → neilio
Assignee

Comment 13

10 years ago
Here's a patch that makes the required changes - it needs the <ul> changes to the HTML to properly work, though -- without that change the layout will shift like crazy.
Attachment #386801 - Attachment is obsolete: true
Attachment #395721 - Flags: review?
Attachment #395721 - Flags: review? → review?(fwenzel)
Assignee

Comment 14

10 years ago
clouserw: I did some testing, and that min-width idea won't fly as it's too invasive -- too many items need to be changed to make it work.

Let's go with this for now and once that php change is in this should at least handle narrow widths a bit better.
Comment on attachment 395721 [details] [diff] [review]
Patch that makes the required CSS changes

Looks good! Committed to r49674, along with making one <ul> per line.
Attachment #395721 - Flags: review?(fwenzel) → review+
Posted image Post-fix screenshot
Here's a final, post-fix screenshot.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: push-needed
Resolution: --- → FIXED
Whiteboard: [patch]
Reporter

Comment 17

10 years ago
Verified on https://preview.addons.mozilla.org/en-US/firefox/browse/type:2/cat:all?sort=popular

Thanks guys.
Status: RESOLVED → VERIFIED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.