Closed Bug 1242003 Opened 9 years ago Closed 6 years ago

"Load More..." row in dominator trees should be styled differently

Categories

(DevTools :: Memory, defect, P3)

Product:
DevTools
Component:
Memory
Type:
defect

Tracking

(firefox67 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 67
Tracking Flags:
Tracking Status
firefox67 --- fixed

People

(Reporter: jsantell, Assigned: lalas, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: good-first-bug, Whiteboard: [good first bug][lang=css][lang=js])

Attachments

(3 files)

Screen Shot 2016-01-21 at 10.33.00 PM.png
(deleted), image/png
Details
Bug 1242003 - Update style of 'Load more...' row in dominator trees on devtools:memory.
(deleted), text/x-review-board-request
Details
Bug 1242003 - Load More row in dominator trees should be styled differently. r=julienw
(deleted), text/x-phabricator-request
Details
Not sure how exactly, but atleast have a `pointer` cursor to make it look clicky.
Has STR: --- → irrelevant
I would like to fix this bug. However this would be my first bug and I have no idea if i have adequate knowledge to debug it . I would try my best howsoever. Please assign this bug to me .
Flags: needinfo?(jsantell)
Assignee: nobody → deepesh910
Status: NEW → ASSIGNED
Since this is my first bug , i need some help . Where do i exactly find the code which i have to de-bug ?please help.
The "load more" frame can be found in devtools/client/memory/components/dominator-tree.js -- the `.load-more` class can be styled in devtools/client/themes/memory.css
Flags: needinfo?(jsantell)
Hello Jordan, I would like to work on this bug. I'll try to submit a patch as soon as I can. Thank You.
[good first bug] whiteboard -> keyword mass change
Keywords: good-first-bug
If this is still open I would love to work on this bug. Would it be possible to be assigned it?
I really want to give this a shot. Can I have this assigned?
Hi! I uploaded a patch for this. I'm not sure if the .load-more class I implemented has the expected behavior, if not, just let me know which properties are expected and I'll add them right away :)
Flags: needinfo?(jordan)
Thank you for the patch! I'm not really sure of the status of this bug, pinging Greg who will have a better idea.
Flags: needinfo?(jordan) → needinfo?(gtatum)
Attachment #8868356 - Flags: review?(jordan)
I would like to work on this bug as part of my university coursework. Please could I be assigned to it and given extra information? Thank you!
(In reply to Rayann Tedds from comment #11) > I would like to work on this bug as part of my university coursework. Please > could I be assigned to it and given extra information? Thank you! Also, how do I actually get to review the bug - is there a HTML file to view or something? Thank you!
Sorry, this fell of my radar. The existing attached patch may actually work for it (although by now it's a kind of stale). This tool is a bit on "maintenance mode" with only fixing regressions. If you're looking for some easy first patches we're actively working on perf.html for performance work in Firefox: https://github.com/devtools-html/perf.html/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22
Flags: needinfo?(gtatum)
Product: Firefox → DevTools

This bug has not been updated in the last 6 months. Resetting the assignee field.
Please, feel free to pick it up again and add a comment outlining your plans for it if you do still intend to work on it.
This is just trying to clean our backlog of bugs and make bugs available for people.

Assignee: deepesh910 → nobody
Status: ASSIGNED → NEW

Hi, I would like to have this bug assigned to me.

According to comment #10, the JS code for the "load more" frame can be found in devtools/client/memory/components/dominator-tree.js and the .load-more class can be modified/added in 'devtools/client/themes/memory.css'.

Thanks!

Hey lloanalas, yes please :)

As comment 0 says, it should get a "pointer" cursor on hover. And probably we should make it look a bit more like a link (as we don't have enough space for a button), so please follow the style outlined in https://design.firefox.com/photon/components/links.html. Maybe we should even make it a link at all?

Assignee: nobody → lloanalas

Julien,

I was reading the style outline and it seems like links serve a different purpose. The 'load more' text does trigger a change in the view - wouldn't that call for the use of a button? Is the issue using a button that the size of the button would be too small? https://i.imgur.com/5tjaiMZ.png

Updated "Load More" row in dominator trees to look more like a link in order to tell it apart from other text and to indicate that the element can be clicked. Not enough space

(In reply to lloanalas from comment #17)

Julien,

I was reading the style outline and it seems like links serve a different purpose. The 'load more' text does trigger a change in the view - wouldn't that call for the use of a button? Is the issue using a button that the size of the button would be too small? https://i.imgur.com/5tjaiMZ.png

Yes, that's a good remark. This was also part of my thinking initially. According to the doc, the minimum height of a button is 24px, and we don't have this space, so we'd need to make it smaller, and I don't think this would be nice in the end. So that's why I recommended to use a link: even if it's not a perfect solution, I think it's the best we can do here.

I'll look at your patch shortly, thanks!

Mentor: jordan → felash

:Julienw

I updated the items you pointed out. Could you take a look when you get a chance?

Thank you!

Flags: needinfo?(felash)

I approved and pushed it, thanks a lot!

Flags: needinfo?(felash)

(In reply to Julien Wajsberg [:julienw] from comment #21)

I approved and pushed it, thanks a lot!

Great! Thanks - learned quite a bit about the branding and where to find those items within the code base. Thanks - very useful!

Pushed by jwajsberg@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/22825edb005f Load More row in dominator trees should be styled differently. r=julienw

https://hg.mozilla.org/mozilla-central/rev/22825edb005f

Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox67: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: