278710 - Using user dropdown instead of text field disturbs the layout

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect)

Description

[Vlad Dascalu]

The checkin from bug 255428 caused white rows to appear in the upper part of the
layout due to the "multiple => 5" thing.

The multiple select thing needs to fall down along the screen without forcing
white spaces in the left components.

Comment 1

[Vlad Dascalu]

Adding the CC list from that bug on this one, maybe someone is interested in
fixing the regression.

Comment 2

[:glob ✱]

Attachment: patch v1

ok, this bug was much harder to fix than i expected, due in part to option
fields affecting the layout in different ways.

i went through many iterations before settling on this fix.  i'm not 100% happy
with nested tables, but this produces by far the cleanest layout and code.

i'm in an ascii diagram mood, so here's where the table nesting happens:

+------------------------+--------------+
| +---------+----------+ | +----------+ |
| | bug     | hardware | | | reporter | |
. .	    .	       . . .	      . .

Comment 3

[:glob ✱]

oh, this patch also (accidently) fixes the layout issues you get if you have
many flags (just look at this bug).

Comment 4

[:glob ✱]

Attachment: patch v1 (diff -uw)

same patch, but with -w for easier reading.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

Any chance I could get before and after screenshots of the change?

Comment 6

[:glob ✱]

Attachment: Before

Comment 7

[:glob ✱]

Attachment: After

Comment 8

[:glob ✱]

note that in the before shot, the gaps between bug, product, and component are
caused by the larger cc field; while the gap between url and summary is caused
by the flag list.

Comment 9

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 183884 [details] [diff] [review]
patch v1

The fixes for both the gaps introduced by 255428 and the ones created by
significant numbers of flags are both welcome fixes, but this patch
additionally moves the plaintext field section (the section with URL, summary,
etc.) upwards to the edge of the field sections above them.  The plaintext
section should remain aligned with the top of the flags section, even if that
means leaving a gap between those sections.

Note that in the future we're going to convert each section to a floating div
such that wide screens display more sections side-by-side while narrow screens
display them all vertically without requiring horizontal scrolling.

Comment 10

[Myk Melez [:myk] [@mykmelez]]

(Incidentally, we're also going to reorganize the sections somewhat so that
fields are logically grouped.)

Comment 11

[:glob ✱]

Attachment: mock

before i hit the code again, is this the layout that you're expecting?

Comment 12

[Myk Melez [:myk] [@mykmelez]]

Yes, that's what I'm looking for.

Comment 13

[:glob ✱]

while trivial to do in photoshop, i suspect it's impossible to do with tables :(

how about i just put an empty row between qa contact and url?

Comment 14

[:glob ✱]

Attachment: patch v2 (tr height="1px")

this version uses tables to achieve the desired layout.

however i'm not happy with what i had to do to acheive this.  most of the table
rows now have a height="1pt" attribute, with the exception of two new rows. 
thankfully ie and ff expand the rows to the minimum height to view the content,
and expand the empty rows to fill the rest of the gap.

it looks different in ie, worse than ff but better than the current layout. 
i'll post a screenshot of how ie lays out the bug with this patch.

if possible i'd prefer to go with the previous patch with an empty row between
qacontact and url.

Comment 15

[:glob ✱]

Attachment: patch v2 IE

Comment 16

[Myk Melez [:myk] [@mykmelez]]

Attachment: example of desired layout approach

Actually I was thinking more of something like this patch, which breaks each
section (i.e. collection of label/field pairs) into its own table, then wraps
the sections in a table that lays them out appropriately.

Besides fixing the alignment bugs, this approach groups sections together
structurally (which will make moving to a flexible section-based layout using
divs easier), and it simplifies the code, since instead of one big complicated
and intricately arranged (and hard to hack without unwanted side-effects; hence
this bug and others) table there are five relatively simple and straightforward
two-column tables enclosed inside a five-cell table for layout.

In other words, each section (except for flags, which generates its own table),
groups its content into a table that looks like this:

+-----------------+
| labels | fields |
+-----------------+
| ...	 | ...	  |
+-----------------+
| ...	 | ...	  |
+-----------------+
| ...	 | ...	  |
+-----------------+

The sections are then wrapped in a table which organizes them according to the
current layout of the page, i.e.:

+-----------------------+-------------------------+---------------------+
|			|			  |			|
|			|			  |			|
|    bug #,		|     hardware, 	  |    reporter,	|
|    product,		|     OS,		  |    CC		|
|    etc.		|     etc.		  |			|
|			|			  |			|
|			|			  |			|
+-----------------------+-------------------------+---------------------+
|						  |			|
|						  |			|
|    QA contact,				  |    Flags		|
|    URL,					  |			|
|    etc.					  |			|
|						  |			|
|						  |			|
+-------------------------------------------------+---------------------+

Perhaps this is what you were going for in your first patch?  If so, that patch
was on the right track.  It just needs to align the bottom two sections (URL
etc. and Flags) with each other.  Take a look at this patch and see if you can
use it to update your earlier patch with that fix.

Comment 17

[:glob ✱]

the main problem with that approach is because the "bug, product, .." fields are
in a different table than the "qa contact, .." fields, the width of the field
label column won't be the same.

Comment 18

[:glob ✱]

Attachment: 5 tables screenshot

myk - here's what it looks like.  in addition to the first column width being
inconsistent, the height of the rows are different.

Comment 19

[Max Kanat-Alexander]

Is this actually going to make 2.20, do you think?

Comment 20

[:glob ✱]

> Is this actually going to make 2.20, do you think?

yes.

/me looks at myk

Comment 21

[Myk Melez [:myk] [@mykmelez]]

I see how the row height varies, but how is the first column width inconsistent?

The thing is, although this creates some UI artifacts of its own, it moves us in
the direction we're ultimately going, in which each of these five sections will
be free floating, such that users with sufficiently wide screens will see them
all next to each other, while users with very narrow screens will see them
stacked up vertically (and most users will see, as they do today, two rows of
two-three sections each).

Comment 22

[:glob ✱]

Attachment: 5 tables patch

Comment 23

[Christian Reis]

I'd admire a patch which made the external table instead a set of divs which
flowed nicely depending on how wide or narrow the page was made. This is one of
my main gripes with Bugzilla: I can't view it at 700px width, which is my
regular browser width, and the only reason for that is the silly reporter,cc
(and now, flags) cell which blows us into horizontal scrolling.

Comment 24

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 186884 [details] [diff] [review]
5 tables patch


>I'd admire a patch which made the external table instead a set of divs which
>flowed nicely depending on how wide or narrow the page was made.

Indeed.  That's where we want to be, and this patch doesn't take us there.  But
it does take us halfway, since it separates sections into distinct blocks, a
necessary prerequisite to laying out those sections using divs.

r=myk, but this seems fairly significant to be going into 2.20 at this date. 
Unfortunately, I don't see a simpler enough fix for the problem, including any
of the previous patches.

Comment 25

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 186884 [details] [diff] [review]
5 tables patch

err, the xstyle debugging code needs to be removed from this patch

Comment 26

[:glob ✱]

Attachment: 5 tables patch without debugging code

without xstyle

Comment 27

[Dave Miller [:justdave]]

Ehh...  I hate doing this stuff this close to release, but let's do it.  This
will probably make it a little easier to maintain, too, since it gets things in
the order you expect in the template.

Comment 28

[Max Kanat-Alexander]

Glob: I just did the checkin myself so that I could control the release process.

Checking in template/en/default/bug/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v  <--
 edit.html.tmpl
new revision: 1.60; previous revision: 1.59
done

Comment 29

[Frédéric Buclin]

*** Bug 250632 has been marked as a duplicate of this bug. ***