Definition terms in list have absolute positioning

[Ryan J Ollos]

The dt elements on the changeset and diff view pages have absolute positioning, so if someone wanted to say, add a left margin #overview { margin-left: 2.5em }, the entire contents of the dl would not be shifted:

—>

The following change seems to make sense, since it removes the absolute left position of the dts, and instead sets the margin-left on the parent to preserve their positioning.

trac/htdocs/css/diff.css

@@ -2 +2 @@
 #prefs fieldset { margin: 1em .5em .5em; padding: .5em 1em 0 }
 
 /* Diff/change overview */
-#overview { line-height: 130%; margin-top: 1em; padding: .5em }
+#overview { line-height: 130%; margin-top: 1em; padding: .5em .5em .5em 0 }
 #overview dt.property {
  font-weight: bold;
  padding-right: .25em;
  position: absolute; /* relies on #content { position: relative } */
- left: 0;
  text-align: right;
  width: 7.75em;
 }

After that change, if we set a margin-left in the custom stylesheet (#overview { margin-left: 2.5em }), we get:

There was some recent adjustments to the left property in [11097] and [11136]. Prior to that, the most recent change was [943].

[Ryan J Ollos]

A few more changes to remove the need for absolute positioning:

trac/htdocs/css/diff.css

@@ -2 +2 @@
 #prefs fieldset { margin: 1em .5em .5em; padding: .5em 1em 0 }
 
 /* Diff/change overview */
-#overview { line-height: 130%; margin-top: 1em; padding: .5em }
+#overview { line-height: 130%; margin-top: 1em; padding: .5em .5em .5em 0 }
 #overview dt.property {
+ float: left;
  font-weight: bold;
- padding-right: .25em;
- position: absolute; /* relies on #content { position: relative } */
- left: 0;
+ padding-right: .75em;
  text-align: right;
  width: 7.75em;
 }
-#overview dd { margin-left: 8em }
 
 #overview .message { padding: 1em 0 1px }
 #overview dd.message p, #overview dd.message ul, #overview dd.message ol,

These changes are based on a suggestion by Eric Ladner on the Bloodhound mailing list, and a review of the CSS for the dl-horizontal class in Bootstrap 2.x.

Prior to this patch, if you wanted to adjust the width of the#overview dt, you'd also have to adjust the margin-left of the #overview dd.

[Jun Omae]

The float: left would break the layout in changeset. See changeset-r12330-20131217T115944.png​ and r12330. I don't think that is good idea to use float: left.

[Jun Omae]

Seems to need #overview dd { margin-left: ... }. Try the following.

trac/htdocs/css/diff.css

@@ -2 +2 @@
 #prefs fieldset { margin: 1em .5em .5em; padding: .5em 1em 0 }
 
 /* Diff/change overview */
-#overview { line-height: 130%; margin-top: 1em; padding: .5em }
+#overview { line-height: 130%; margin-top: 1em; padding: .5em .5em .5em 0 }
 #overview dt.property {
+ float: left;
+ clear: left;
  font-weight: bold;
- padding-right: .25em;
- position: absolute; /* relies on #content { position: relative } */
- left: 0;
  text-align: right;
  width: 7.75em;
 }
-#overview dd { margin-left: 8em }
+#overview dd { margin-left: 8.5em }
 
 #overview .message { padding: 1em 0 1px }
 #overview dd.message p, #overview dd.message ul, #overview dd.message ol,

[Ryan J Ollos]

That makes sense I guess since the Bootstrap CSS also adds a margin-left on the dd. Thanks for spotting the need for clear: left as well.

I reviewed a few dozen additional changesets and it looks good for all those cases.

[Ryan J Ollos]

After reviewing changeset.css, it seems there are some rules that aren't used anywhere (or any longer), and could therefore be removed on the trunk. Maybe I'm overlooking where they are used though. Proposed changes in log:rjollos.git:t11403.

[Ryan J Ollos]

I've attempted to trace the history of the CSS that is being removed, to try and reduce the chance of making an error here.

[ecbae66a/rjollos.git]: There is no descendant dl from either of the elements with a #title id: branches/1.0-stable/trac/versioncontrol/templates/diff_form.html?rev=11308​, branches/1.0-stable/trac/versioncontrol/templates/changeset.html?rev=11308​. The CSS was added in [1767#file4] and applicable to a div#title in the diff.cs, which later became changeset.cs [2774]. It looks like the dl markup was removed in [1770].

$ grep -R "id=\"title\"" --exclude-dir=.svn
trac/ticket/templates/report_edit.html:            <input type="text" id="title" name="title" class="trac-focus" value="$report.title" size="50" /><br />
trac/versioncontrol/templates/diff_form.html:      <div id="title">
trac/versioncontrol/templates/changeset.html:      <div id="title" py:choose="" py:with="

I'll add a comment on [e85cf960/rjollos.git] later on.

[Ryan J Ollos]

[e85cf960/rjollos.git]: CSS was added in [1109]. It looks like there was a nested div in the node_change function at that time, <div class="<?cs var:cl ?>"><div class="mod"></div></div>, but it's no longer present, <div class="$cl"> </div>: branches/1.0-stable/trac/versioncontrol/templates/changeset.html@11308:99#L93​.

There are no div descendants from the #overview tag in diff_view.html, which is the other template with a #overview id.

$ grep -R id=\"overview\" --exclude-dir=.svn
trac/templates/diff_view.html:      <dl id="overview" py:with="multi = num_changes &gt; 1">
trac/versioncontrol/templates/changeset.html:      <dl id="overview">

[Jun Omae]

I've investigated [e85cf960/rjollos.git] and [ecbae66a/rjollos.git], too. The two changes look good to me.

[Ryan J Ollos]

Refactoring committed to 1.0-stable in [12349] and merged to trunk in [12350]. CSS removal committed to trunk in [12351].

Log message for [12349] is incorrect and should say Also removed an unused CSS rule from changeset.css. Log message for [12350] is incorrect, and should reference [12349] rather than [12340].