Closed Bug 769446 Opened 10 years ago Closed 9 years ago

"add a comment" link gets pushed off screen when "open reftest analyzer" is displayed

Categories

(Tree Management Graveyard :: TBPL, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mbrubeck, Assigned: ewong)

References

Details

(Keywords: polish)

Attachments

(1 file, 2 obsolete files)

Maybe we can move the reftest analyzer link to the right-hand side of the footer?
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Attachment #654964 - Flags: review?(mbrubeck)
Attachment #654964 - Attachment is obsolete: true
Attachment #654964 - Flags: review?(mbrubeck)
Attachment #654966 - Flags: review?(mbrubeck)
Comment on attachment 654966 [details] [diff] [review]
Moved "open reftest analyzer" to the right, above the duration display. (v2)

Works for me.  Seeking a second opinion in case there's anything I haven't thought of.
Attachment #654966 - Flags: review?(mbrubeck)
Attachment #654966 - Flags: review?(bmo)
Attachment #654966 - Flags: review+
Comment on attachment 654966 [details] [diff] [review]
Moved "open reftest analyzer" to the right, above the duration display. (v2)

Review of attachment 654966 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for doing this :-)
Does the job locally - am giving r+ - please can if you could upload an updated patch with these few bits addressed and I'll land it.

::: css/style.css
@@ +476,5 @@
> +.reftestanalyzer {
> +  position: absolute;
> +  right: 5px;
> +  bottom: 20px;
> +}

To be consistent with the rest of the stylesheet, I think this should be .reftestAnalyzer.

To make the link line up with 'add a comment' to the left, this needs to be:
bottom: 26px;

::: js/UserInterface.js
@@ +1509,4 @@
>            '                   function() { $("p").removeClass("loading").text("Fetching reftest log failed.") },\n' +
>            '                   function() { $("p").removeClass("loading").text("Fetching reftest log timed out.") });\n' +
>            '</script>';
> +        return '<span class="reftestanalyzer"><a href="data:text/html,' + escape(html) + '">open reftest analyzer</a></span>';

Whilst this works as-is, I think the code might be more readable, if the order was changed to match where they appear in the UI. ie: the whole htmlForReftests function should go after htmlForAddComment() and before htmlForDuration().
Attachment #654966 - Flags: review?(bmo) → review+
Fixed nits.
Attachment #654966 - Attachment is obsolete: true
Attachment #655986 - Flags: review+
I had to back this out and reland, since I didn't spot there wasn't an author set in the patch (since the previous patches in other bugs had it filled out). Please can you adjust your hgrc so this is added automatically :-)
https://developer.mozilla.org/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Landed as:
http://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/rev/3631325e489e
Depends on: 790559
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Product: Webtools → Tree Management
Product: Tree Management → Tree Management Graveyard
You need to log in before you can comment on or make changes to this bug.