-
-
reviewboard/htdocs/media/rb/css/reviews.css (Diff revision 1) It seems like the border styles are repeated 3 times. Is there a way to combine these?
Added javascript to uncollapse message issues before navigating to them. (Would not navigate if collapsed before)
Review Request #2646 — Created Oct. 6, 2011 and discarded
Information | |
---|---|
medanat | |
Review Board | |
Reviewers | |
reviewboard | |
Added javascript to uncollapse message issues before navigating to them. (Would not navigate if collapsed before) -Added .click listener to issue descriptions in table. -Added an id attribute to each issue in order to be able to locate them using jquery. Fixes: -Changed <b> to <strong> -Added trans blocks around every static piece of text. -Redistributed the CSS classes based on Dave Druska and Mike Conley's recommendation. -Alphabetized the ordering of CSS rules. -Double space indentation for rules. CSS enhancements to Issue Summary Table. Issue Summary Table starts collapsed. Increased anchor width to 100%. Added :hover coloring. Added left-most and right-most borders. Fixed padding on Issue Summary Table. Removed Table if no issues are found. Fixes: Removed table entirely if no issues are found. Removed table row spacing. Added blue border color for :hover event. Removed unnecessary CSS rules.
Linux (Ubuntu 11.04): FF 7.01, Chromium 12.
Description | From | Last Updated |
---|---|---|
It seems like the border styles are repeated 3 times. Is there a way to combine these? |
DD ddruska | |
These should be in alphabetical order. |
|
|
alphabetical order |
|
|
alphabetical order |
|
|
alphabetical order |
|
|
Also, these should probably be in translation blocks. |
|
|
Should be ".issue .open" |
|
|
Same as above, ".issue .dropped" |
|
|
Same as above, ".issue .resolved" |
|
|
Two space indentation, as opposed to one |
|
-
Yazan: Looks good! Just a few notes below. Thanks, -Mike
-
reviewboard/htdocs/media/rb/css/reviews.css (Diff revision 1) Alternatively, perhaps we could have .issue be a class on its own, and .open, .dropped, .resolved be classes too. Then we could just do: .issue .status { border-right-width: 1px; } .issue .description { border-left-width: 1px; } .issue .resolved td { /* blah */ } etc. Just an idea - though might be more trouble than it's worth.
-
reviewboard/htdocs/media/rb/css/reviews.css (Diff revision 1) These should be in alphabetical order.
-
-
-
-
reviewboard/templates/reviews/review_detail.html (Diff revision 1) I think <strong> now is preferred over <b>
-
reviewboard/templates/reviews/review_detail.html (Diff revision 1) Also, these should probably be in translation blocks.
Change Summary:
-Changed <b> to <strong> -Added trans blocks around every static piece of text. -Redistributed the CSS classes based on Dave Druska and Mike Conley's recommendation. -Alphabetized the ordering of CSS rules.
Summary: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Testing Done: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 2 (+63 -39) |
-
A couple of tiny problems snuck in, but this is looking pretty good. We might want to double-check with Christian to ensure that the LessCSS stuff that just went in won't impact this work. -Mike
-
-
-
-
reviewboard/htdocs/media/rb/css/reviews.css (Diff revision 2) Two space indentation, as opposed to one
Change Summary:
CSS rule indentation fix.
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+63 -39) |
-
My lesscss changes break this. The .css files are gone. You'll need to sync and move your changes over to the .less files. Also, your change deception shows you haven't merged in master. Make sure the description is updated when you do.
-
Yazan: This looks good - I just have one final thing I found. -Mike
-
reviewboard/templates/reviews/review_detail.html (Diff revision 3) Hm - it might be better to set the name / id of this element to by something like STRINGPREFIX-comment.id, just for clarity's sake. Same applies below. Also, this will help make it so that we don't get into any conflicts if Comment / Screenshot / Attachment issue comments have the same ID # (which is an unfortunate consequence of storing each of those models in different tables).