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

VERIFIED FIXED in 5.0.9

Status

VERIFIED FIXED
10 years ago
3 years ago

People

(Reporter: Milos, Assigned: neilio)

Tracking

unspecified
5.0.9

Details

(URL)

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
Last Resolved: 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
Keywords: push-needed
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.