Closed Bug 1473211 Opened 6 years ago Closed 6 years ago

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
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: