Closed Bug 1473211 Opened Last year Closed Last year

Stop using string concatenation for the string IDs in the meatball menu

Categories

(DevTools :: General, enhancement, P3)

enhancement

Tracking

(firefox63 fixed)

RESOLVED FIXED
Firefox 63
Tracking Status
firefox63 --- fixed

People

(Reporter: birtles, Assigned: birtles)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

See bug 1461522 comment 151:

> ::: devtools/client/framework/components/MeatballMenu.js:101
> (Diff revision 6)
> > +    const items = [];
> > +
> > +    // Dock options
> > +    for (const hostType of this.props.hostTypes) {
> > +      const l10nkey =
> > +        hostType.position === "window" ? "separateWindow" : hostType.position;
> 
> This code was just moved so I don't want to block landing on this, but in
> general I prefer to avoid string concatenation for l10n keys. More verbose,
> but we really have no tooling to find where a given l10n string is used,
> unless the code uses the exact same string.
> 
> Also this kind of code tends to become messy really quickly. As soon as a
> string is updated and we add the typical "2" suffix, this kind of code
> usually morphs into `if (type === "something") { key = key + "2"}`. Example:
> https://searchfox.org/mozilla-central/source/devtools/client/storage/ui.
> js#992-995
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
Using the full string each time, although more verbose, allows us to easily
search for string IDs and see where they are used.
Comment on attachment 8996599 [details]
Bug 1473211 - Stop using string concatenation for string IDs in MeatballMenu; r=jdescottes

Julian Descottes [:jdescottes][:julian] has approved the revision.

https://phabricator.services.mozilla.com/D2590
Attachment #8996599 - Flags: review+
Pushed by bbirtles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/aa9e4ca3dbca
Stop using string concatenation for string IDs in MeatballMenu; r=jdescottes
https://hg.mozilla.org/mozilla-central/rev/aa9e4ca3dbca
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in before you can comment on or make changes to this bug.