Closed
Bug 1336207
Opened 7 years ago
Closed 7 years ago
Add tooltip to say what "ev" means
Categories
(DevTools :: Inspector, defect, P3)
Tracking
(firefox57 verified)
VERIFIED
FIXED
Firefox 57
Tracking | Status | |
---|---|---|
firefox57 | --- | verified |
People
(Reporter: jidanni, Assigned: aastha.gupta4104, Mentored)
Details
(Keywords: good-first-bug)
Attachments
(1 file, 2 obsolete files)
3.76 KB,
patch
|
miker
:
review+
|
Details | Diff | Splinter Review |
One sees "Button: [ev]" but there is not tooltip to pop up and say what "ev" means or might do.
Reporter | ||
Comment 1•7 years ago
|
||
Same with Bubbing and Domo.
Reporter | ||
Comment 2•7 years ago
|
||
Their current tooltips are just their names. That can never be right.
Comment 3•7 years ago
|
||
Good point, the (ev) icon should definitely have a title tooltip so people know what it is about. And the DOM0/1/2 and Bubbling tags just have silly tooltips. This is an easy bug if someone wants to work on it. I'm happy to help. Make sure you go over our contribution guide first: https://developer.mozilla.org/en-US/docs/Tools/Contributing Once ready, you might be interested in looking at the following code locations: /devtools/client/inspector/markup/markup.xhtml, in this file, the div.markupview-events element is the little (ev) icon in the inspector. Adding a title attribute to it would work fine. In devtools/client/shared/widgets/tooltip/EventTooltipHelper.js we create the content of the event tooltips, including the little DOM0/1/2 and Bubbling tags. Here's the exact code position where the title attribute is added: http://searchfox.org/mozilla-central/rev/b1aadb3572eaf7d2c70e19a2ba5413809d9ac698/devtools/client/shared/widgets/tooltip/EventTooltipHelper.js#137 As you can see, the title is just the same text as the tag itself. We should introduce an actual localized string for this instead. devtools/client/locales/en-US/inspector.properties is the properties file that contains our inspector localized strings. So it would be the right place to add the new titles. Let me know if you need more help.
Mentor: pbrosset
Comment 4•7 years ago
|
||
Hi Patrick, Thanks for mentoring, I will submit a patch soon, assigning for now, and will let you know if I need anything. Thanks
Assignee: nobody → 3ugzilla
Status: NEW → ASSIGNED
Hi, This is my first patch in Mozilla community.Kindly review it and let me know if any changes are required. Thanks
Comment 7•7 years ago
|
||
Thank you very much for getting involved on the project! Your code changes look pretty good at first glance. I'm going to pass mentorship of this bug to Mike who knows this part better than I do and will be able to help you and review your code changes. It would also be good to update the DOM0/1/2/ and Bubbling tooltips, I don't see this being changed in your patch. Anyway, great start. I'll assign the bug to you now.
Assignee: 3ugzilla → aastha.gupta4104
Mentor: pbrosset → mratcliffe
Updated•7 years ago
|
Attachment #8905288 -
Flags: review?(mratcliffe)
(In reply to Aastha from comment #6) > Hi, > This is my first patch in Mozilla community.Kindly review it and let me know > if any changes are required. > Thanks Nice work, thanks for contributing. Do you think you could also remove the DOM0/1/2/ and Bubbling tooltips then re-upload the patch?
Flags: needinfo?(aastha.gupta4104)
Attachment #8905288 -
Attachment is obsolete: true
Attachment #8905288 -
Flags: review?(mratcliffe)
Flags: needinfo?(aastha.gupta4104)
I didn't realize that you were localizing the tooltips. Just a couple of changes and you are done. Can you create a single patch with the following changes? 1. Add a tooltip to the ev button. 2. Remove the tooltips from the DOM0 and DOM2 tags because they are the same in every language. 3. Localize the tooltips for Capturing and Bubbling (move them to the .properties file).
Flags: needinfo?(aastha.gupta4104)
Assignee | ||
Comment 11•7 years ago
|
||
1. Added a tooltip to the ev button. 2. Removed the tooltips from the DOM0 and DOM2 tags. 3. Localized the tooltips for Capturing and Bubbling.
Attachment #8905506 -
Attachment is obsolete: true
Flags: needinfo?(aastha.gupta4104)
Comment on attachment 8906343 [details] [diff] [review] Made all the changes that had to be made Review of attachment 8906343 [details] [diff] [review]: ----------------------------------------------------------------- Perfect, great job!
Attachment #8906343 -
Flags: review+
Keywords: checkin-needed
Comment 13•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/336effc1fe44 Add tooltip to say what "ev" means in the Inspector. r=miker
Keywords: checkin-needed
Comment 14•7 years ago
|
||
Pushed by archaeopteryx@coole-files.de: https://hg.mozilla.org/integration/mozilla-inbound/rev/0e2f9e7b7fd7 Add tooltip to say what "ev" means in the Inspector. Follow-up: Fix long line. r=eslint-fix
Comment 15•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/336effc1fe44 https://hg.mozilla.org/mozilla-central/rev/0e2f9e7b7fd7
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Comment 16•7 years ago
|
||
I have reproduced this Bug with nightly 54.0a1 (2017-02-02) on Ubuntu 16.04, 64 Bit! The bug's fix is now verified on latest nightly 57.0a1 Build ID 20170912100139 User Agent Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0
QA Whiteboard: [bugday-20170913]
Comment 17•7 years ago
|
||
I have successfully reproduced this bug with Nightly 57.0a1 (2017-02-02) on windows 10 (32-bit) this bug is verified fix with latest nightly 57.0a1 (2017-09-12) (32-bit) Build ID: 20170912100139 Mozilla/5.0 (Windows NT 10.0; rv:57.0) Gecko/20100101 Firefox/57.0 [bugday-20170913]
Comment 18•7 years ago
|
||
As per Comment 16 and Comment 17, I am marking this bug as verified fixed.
Status: RESOLVED → VERIFIED
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•