Implement dynamic fetching of actions for submenu_button <menulist>
Categories
(Firefox :: Messaging System, task, P3)
Tracking
()
People
(Reporter: emcminn, Unassigned)
References
(Blocks 2 open bugs)
Details
Attachments
(1 file)
|
200.03 KB,
image/png
|
Details |
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.
Comment 1•2 years ago
|
||
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.
Comment 2•2 years ago
|
||
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:
fluentLabeltol10n-idandrawTexttolabelto keep a consistent naming style with notificationbox.js- the order of arguments to make the optional parameter
rawText(orlabel) the last parameter - to avoid the comment:
currentSelectiontoselectedValue
{
selectedValue: string,
items: array of objects with {"value", "iconPath", "l10n-id", "label"} // l10n-id will be used if present, label otherwise
}
Updated•2 years ago
|
Comment 3•2 years ago
•
|
||
(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:
fluentLabeltol10n-idandrawTexttolabelto 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(orlabel) the last parameter- to avoid the comment:
currentSelectiontoselectedValue
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.
Updated•2 years ago
|
| Reporter | ||
Updated•2 years ago
|
Comment 4•2 years ago
|
||
@mpohle is working on this over the next week or two.
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
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.
Updated•1 year ago
|
Description
•