Flexbox-Toggle misses a tooltip
Categories
(DevTools :: Inspector, defect, P3)
Tracking
(firefox67 fixed)
Tracking | Status | |
---|---|---|
firefox67 | --- | fixed |
People
(Reporter: nachtigall, Assigned: championshuttler, Mentored)
References
(Blocks 1 open bug)
Details
(Keywords: good-first-bug)
Attachments
(2 files)
Updated•6 years ago
|
Comment 1•6 years ago
|
||
Thanks for reporting this! I think this is a good first bug and am willing to mentor anyone interested in it.
As Jens said, this can be solved by adding the title attribute to the input element rendered in devtools/client/inspector/flexbox/components/Header.js
. Specifically, renderFlexboxHighlighterToggle
is responsible for rendering the Flexbox Toggle component, which is where we can add a title attribute saying "Toggles Flexbox Highlighter".
Updated•6 years ago
|
Assignee | ||
Comment 2•6 years ago
|
||
Hi Micah, I would like to work on it can you help me getting started?
Thanks
Comment 3•6 years ago
|
||
Hi Shivam, thanks for picking this up! To get started, you can read through the "Getting Started" and "Contributing" sections at https://docs.firefox-dev.tools/. After that, feel free to ask any questions you have!
Assignee | ||
Comment 4•6 years ago
|
||
Hi Micah, so i am done with setting up gecko locally on my laptop , can you help me ahead how to do that , I am unable to figure it out.
Thanks
Comment 5•6 years ago
|
||
Hi Shivam! Could you tell me which part of the setup process you are having trouble with? That way we can figure out where to start.
Assignee | ||
Comment 6•6 years ago
|
||
Hi Micah, I am done with setup but i need help with how to add the title in renderFlexboxHighlighterToggle
at devtools/client/inspector/flexbox/components/Header.js
Thanks
Assignee | ||
Comment 7•6 years ago
|
||
As we need to add title attribute to input element so can we do something like
renderFlexboxHighlighterToggle() {
// Don't show the flexbox highlighter toggle for the parent flex container of the
// selected element.
if (this.props.flexContainer.isFlexItemContainer) {
return null;
}
return createElement(Fragment, null,
dom.div({ className: "devtools-separator" }),
dom.input({
id: "flexbox-checkbox-toggle",
className: "devtools-checkbox-toggle",
checked: this.props.highlighted,
onChange: this.onFlexboxCheckboxClick,
+ title: "Toggles Flexbox Highlighter",
type: "checkbox",
})
);
}
Comment 8•6 years ago
|
||
Yes! This is correct :)
Looking at this now, I realize we would need to ensure that our tooltip can be localized. I apologize for not mentioning this before. So in order to do this, we can do this by adding a unique translation key and the text Toggles Flexbox Highlighter
to devtools/client/locales/en-US/layout.properties
like so:
# LOCALIZATION NOTE (flexbox.togglesFlexboxHighlighter): The tooltip text for the Flexbox toggle button.
flexbox.togglesFlexboxHighlighter=Toggles Flexbox Highlighter
Then once we have done this, we can use getStr
to use that translation key we created and retrieve the localized text. So, first, we would need to import getStr
into devtools/client/inspector/flexbox/components/Header.js
. Like this:
const { getStr } = require("devtools/client/inspector/layout/utils/l10n");
Now that we have getStr
, we can use it like so:
> renderFlexboxHighlighterToggle() {
> // Don't show the flexbox highlighter toggle for the parent flex container of the
> // selected element.
> if (this.props.flexContainer.isFlexItemContainer) {
> return null;
> }
>
> return createElement(Fragment, null,
> dom.div({ className: "devtools-separator" }),
> dom.input({
> id: "flexbox-checkbox-toggle",
> className: "devtools-checkbox-toggle",
> checked: this.props.highlighted,
> onChange: this.onFlexboxCheckboxClick,
> + title: getStr("flexbox.togglesFlexboxHighlighter"),
> type: "checkbox",
> })
> );
> }
Once this is done, our title attribute can be localized! Please let me know if something is unclear, or if you need any more help for this part. I will also make sure that you're assigned to this bug.
Also, in case you're interested, you can find more information on localization in the Firefox project here: https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Create_localizable_strings
Updated•6 years ago
|
Assignee | ||
Comment 9•6 years ago
|
||
Add tooltop in Flexbox toggle
Assignee | ||
Comment 10•6 years ago
|
||
Awesome thanks for the help @Micah, yeah I did this localizable string thing before , thank you so much for the help on Sunday as well :). Have a good day!
Created the patch.
Updated•6 years ago
|
Comment 11•6 years ago
|
||
Comment 12•6 years ago
|
||
bugherder |
Comment 13•6 years ago
|
||
Please note that tooltips are usually written in infinitive mood to describe an action the user can choose, not "what it does". That would mean "Toggle Flexbox Highlighter" in this case.
Comment 14•6 years ago
|
||
Hi Shivam, would you be interested in making this fix? You will need to change the ID from "flexbox.togglesFlexboxHighlighter" to "flexbox.togglesFlexboxHighlighter2". We can't make changes to the existing localization without bumping the id so that it will be picked up by localization team.
Assignee | ||
Comment 15•6 years ago
|
||
Hi Gabriel, thanks for asking sure I will send the patch soon :)
Thanks
Assignee | ||
Comment 16•6 years ago
|
||
Comment 17•6 years ago
|
||
Comment 18•6 years ago
|
||
bugherder |
Description
•