Open Bug 1882833 Opened 2 years ago Updated 1 year ago

Implement dynamic fetching of actions for submenu_button <menulist>

Categories

(Firefox :: Messaging System, task, P3)

task

Tracking

()

People

(Reporter: emcminn, Unassigned)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

The submenu_button <menulist> should be populated via a call to a custom function that the Desktop Integrations team will write. It will take the form:

function populateMailtoProtocolHandlers();

And will return an object of the form:

{
currentSelection: string, // value
items: array of objects with {iconPath, fluentLabel, rawText, value} // fluentLabel will be used if present, rawText otherwise
}
for fetching the list of items dynamically, rather than a static list of menuitems in the submenu property: we just need to add some JSON property/value that will cause the component to use the aforementioned hook to get the list of items.

Hey Michael, is there any update on the populator function? If that work is being tracked in a bug, we should probably add it as a blocker. The exact form of the response is probably going to inform the way we write this patch.

Flags: needinfo?(mhughes)

Hi! As I am going to develop against the new API call, I think the suggested format above already looks good as the iconPath will only contain strings. The prototype I created for that tries to find tippyTopIcons first, with a chrome:// URL and falls back to page-icon://. I think it makes sense to limit the iconPath to use either of the both internal protocols and the fallback mechanism has still some problems and should not become a part of the messaging system for that reason.

I have no strong opinion about that and will respect any decision you make based on what fits the best into the messaging system, but please consider to change:

  • fluentLabel to l10n-id and rawText to label to keep a consistent naming style with notificationbox.js
  • the order of arguments to make the optional parameter rawText (or label) the last parameter
  • to avoid the comment: currentSelection to selectedValue
{
selectedValue: string,
items: array of objects with {"value", "iconPath", "l10n-id", "label"} // l10n-id will be used if present, label otherwise
}
Flags: needinfo?(shughes)
Flags: needinfo?(emcminn)
Attached image API schema suggestion

(In reply to Max from comment #2)

I have no strong opinion about that and will respect any decision you make based on what fits the best into the messaging system, but please consider to change:

  • fluentLabel to l10n-id and rawText to label to keep a consistent naming style with notificationbox.js

notificationbox isn't involved in Spotlight dialogs. We use React to render Spotlights. Localization is currently done through this component. Since we're hoping to extend this SubmenuButton component to support menulists, rather than creating a new component, this gives us a good starting point. And since we're the API's only consumers, it makes sense for the API to use the same property keys as SubmenuButton, so we won't have to add additional code to transform the API's return value.

  • the order of arguments to make the optional parameter rawText (or label) the last parameter
  • to avoid the comment: currentSelection to selectedValue

I agree currentSelection is awkward, but can we use value instead? Aside from using the SubmenuButton properties for localization, it also seems logical to follow the menulist pattern for any other properties, since we are ultimately going to turn the return value into a menulist. For MozMenuList, there are two similar properties: value and selectedItem. value is always a string or undefined, while selectedItem is intended to be a reference to the selected MozMenuItem. selectedValue sounds like both of them, so it could be confusing.

See attached image for my suggestion.

Edit: Also, your suggestion regarding icons sounds good. We'll trust your judgment on how to handle the icons.

Flags: needinfo?(shughes)
Iteration: --- → 125.2 - Mar 4 - Mar 15
Flags: needinfo?(emcminn)

@mpohle is working on this over the next week or two.

Flags: needinfo?(mhughes) → needinfo?(mpohle)
Iteration: 125.2 - Mar 4 - Mar 15 → 126.1 - Mar 18 - Mar 29
Iteration: 126.1 - Mar 18 - Mar 29 → 127.1 - Apr 15 - Apr 26

This is currently on hold.

I spoke to Max and we've decided to work on an experiment with the info bar version to determine if we should even pursue this option.

Flags: needinfo?(mpohle)
Priority: P1 → P3
Iteration: 127.1 - Apr 15 - Apr 26 → ---
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: