Bug 1827943 - Implement the weather suggestion result menu UI. r=dao,fluent-reviewers,flod

This implements the weather suggestion result menu UI and builds on D174941.
References:

* [Spec]( https://www.figma.com/file/Hdi0oHB7trRcncyVAKZypO/accuweather-explorations?node-id=2421%3A62540&t=29w6wH3UYchqBxqX-1) (See "A11y review" in the sidebar)
* [Clickable prototype](https://www.figma.com/proto/Hdi0oHB7trRcncyVAKZypO/accuweather-explorations?page-id=2192%3A42825&node-id=2394-52468&viewport=246%2C526%2C0.12&scaling=min-zoom&starting-point-node-id=2394%3A52468&show-proto-sidebar=1) (See "Revised 4/3" in the sidebar)

There are a couple important points about the menu. First, one of the commands,
"Report inaccurate location", is specific to weather suggestions, or at least
location-based suggestions. I don't think it's a good idea to centralize all
commands in UrlbarView, and in general I'd like to stop centralizing handling of
different result types in the view and input, so I added a new provider method
called `getResultCommands()`.

Second, the spec calls for a menu separator and a submenu so the user can select
a reason they don't want to see the result, so the return value of
`getResultCommands()` is flexible enough to support those two things, and I
modified `#populateResultMenu()` too.

These new commands will be recorded in Glean engagement telemetry as new
`engagement_type` values, same as "dismiss" and "help" currently are.

This patch doesn't implement handling of two of the commands, "Report inaccurate
location" and "Show less frequently", because I wanted to keep it focused on the
fundamentals described above.

Depends on D174941

Differential Revision: https://phabricator.services.mozilla.com/D174994
This commit is contained in:
Drew Willcoxon 2023-04-18 16:22:56 +00:00
parent 76439c38d3
commit 6829a78aa2
7 changed files with 196 additions and 38 deletions

View File

@ -29,6 +29,14 @@ const TELEMETRY_SCALARS = {
IMPRESSION: `${TELEMETRY_PREFIX}.impression_weather`,
};
const RESULT_MENU_COMMAND = {
HELP: "help",
INACCURATE_LOCATION: "inaccurate_location",
NOT_INTERESTED: "not_interested",
NOT_RELEVANT: "not_relevant",
SHOW_LESS_FREQUENTLY: "show_less_frequently",
};
const WEATHER_DYNAMIC_TYPE = "weather";
const WEATHER_VIEW_TEMPLATE = {
attributes: {
@ -235,6 +243,8 @@ class ProviderWeather extends UrlbarProvider {
url: suggestion.url,
iconId: suggestion.current_conditions.icon_id,
helpUrl: lazy.QuickSuggest.HELP_URL,
// TODO: Remove helpL10n, isBlockable, and blockL10n once the telemetry
// test is updated for the result menu.
helpL10n: {
id: lazy.UrlbarPrefs.get("resultMenu")
? "urlbar-result-menu-learn-more-about-firefox-suggest"
@ -267,6 +277,49 @@ class ProviderWeather extends UrlbarProvider {
this.#resultFromLastQuery = result;
}
getResultCommands(result) {
return [
{
name: RESULT_MENU_COMMAND.INACCURATE_LOCATION,
l10n: {
id: "firefox-suggest-weather-command-inaccurate-location",
},
},
{
name: RESULT_MENU_COMMAND.SHOW_LESS_FREQUENTLY,
l10n: {
id: "firefox-suggest-weather-command-show-less-frequently",
},
},
{
l10n: {
id: "firefox-suggest-weather-command-dont-show-this",
},
children: [
{
name: RESULT_MENU_COMMAND.NOT_RELEVANT,
l10n: {
id: "firefox-suggest-weather-command-not-relevant",
},
},
{
name: RESULT_MENU_COMMAND.NOT_INTERESTED,
l10n: {
id: "firefox-suggest-weather-command-not-interested",
},
},
],
},
{ name: "separator" },
{
name: RESULT_MENU_COMMAND.HELP,
l10n: {
id: "urlbar-result-menu-learn-more-about-firefox-suggest",
},
},
];
}
/**
* This is called only for dynamic result types, when the urlbar view updates
* the view of one of the results of the provider. It should return an object
@ -394,14 +447,13 @@ class ProviderWeather extends UrlbarProvider {
}
}
// Handle dismissals.
if (
details.result?.providerName == this.name &&
details.selType == "dismiss"
) {
this.logger.info("Dismissing weather result");
lazy.UrlbarPrefs.set("suggest.weather", false);
queryContext.view.controller.removeResult(details.result);
// Handle commands.
if (details.result?.providerName == this.name) {
this.#handlePossibleCommand(
queryContext,
details.result,
details.selType
);
}
this.#resultFromLastQuery = null;
@ -480,9 +532,7 @@ class ProviderWeather extends UrlbarProvider {
break;
default:
if (selType) {
this.logger.error(
"Engagement telemetry error, unknown selType: " + selType
);
eventObject = "other";
}
break;
}
@ -505,6 +555,28 @@ class ProviderWeather extends UrlbarProvider {
);
}
#handlePossibleCommand(queryContext, result, selType) {
switch (selType) {
case RESULT_MENU_COMMAND.HELP:
// "help" is handled by UrlbarInput, no need to do anything here.
break;
// selType == "dismiss" when the user presses the dismiss key shortcut.
case "dismiss":
case RESULT_MENU_COMMAND.NOT_INTERESTED:
case RESULT_MENU_COMMAND.NOT_RELEVANT:
this.logger.info("Dismissing weather result");
lazy.UrlbarPrefs.set("suggest.weather", false);
queryContext.view.controller.removeResult(result);
break;
case RESULT_MENU_COMMAND.INACCURATE_LOCATION:
// TODO
break;
case RESULT_MENU_COMMAND.SHOW_LESS_FREQUENTLY:
// TODO
break;
}
}
// The result we added during the most recent query.
#resultFromLastQuery = null;
}

View File

@ -2318,6 +2318,35 @@ export class UrlbarProvider {
return null;
}
/**
* Gets the list of commands that should be shown in the result menu for a
* given result from the provider. All commands returned by this method should
* be handled by implementing `onEngagement()` with the possible exception of
* commands automatically handled by the urlbar, like "help".
*
* @param {UrlbarResult} result
* The menu will be shown for this result.
* @returns {Array}
* If the result doesn't have any commands, this should return null.
* Otherwise it should return an array of command objects that look like:
* `{ name, l10n, children}`
*
* {string} name
* The name of the command. Must be specified unless `children` is
* present. When a command is picked, its name will be passed as
* `details.selType` to `onEngagement()`. The special name "separator"
* will create a menu separator.
* {object} l10n
* An l10n object for the command's label: `{ id, args }`
* Must be specified unless `name` is "separator".
* {array} children
* If specified, a submenu will be created with the given child commands.
* Each object in the array must be a command object.
*/
getResultCommands(result) {
return null;
}
/**
* Defines whether the view should defer user selection events while waiting
* for the first result from this provider.

View File

@ -2671,8 +2671,8 @@ export class UrlbarView {
/**
* @param {UrlbarResult} result
* The result to get menu commands for.
* @returns {Map}
* Map of menu commands available for the result, null if there are none.
* @returns {Array}
* Array of menu commands available for the result, null if there are none.
*/
#getResultMenuCommands(result) {
if (!lazy.UrlbarPrefs.get("resultMenu")) {
@ -2681,46 +2681,72 @@ export class UrlbarView {
if (this.#resultMenuCommands.has(result)) {
return this.#resultMenuCommands.get(result);
}
let commands = new Map();
let commands = lazy.UrlbarProvidersManager.getProvider(
result.providerName
).tryMethod("getResultCommands", result);
if (commands) {
this.#resultMenuCommands.set(result, commands);
return commands;
}
commands = [];
if (
result.source == lazy.UrlbarUtils.RESULT_SOURCE.HISTORY &&
!result.autofill
) {
commands.set(RESULT_MENU_COMMANDS.DISMISS, {
l10n: { id: "urlbar-result-menu-remove-from-history" },
});
commands.set(RESULT_MENU_COMMANDS.HELP, {
l10n: { id: "urlbar-result-menu-learn-more" },
});
commands.push(
{
name: RESULT_MENU_COMMANDS.DISMISS,
l10n: { id: "urlbar-result-menu-remove-from-history" },
},
{
name: RESULT_MENU_COMMANDS.HELP,
l10n: { id: "urlbar-result-menu-learn-more" },
}
);
}
if (result.payload.isBlockable) {
commands.set(RESULT_MENU_COMMANDS.DISMISS, {
commands.push({
name: RESULT_MENU_COMMANDS.DISMISS,
l10n: result.payload.blockL10n,
});
}
if (result.payload.helpUrl) {
commands.set(RESULT_MENU_COMMANDS.HELP, {
commands.push({
name: RESULT_MENU_COMMANDS.HELP,
l10n: result.payload.helpL10n,
});
}
let rv = commands.size ? commands : null;
let rv = commands.length ? commands : null;
this.#resultMenuCommands.set(result, rv);
return rv;
}
#populateResultMenu() {
this.resultMenu.textContent = "";
for (const [command, data] of this.#getResultMenuCommands(
this.#resultMenuResult
)) {
let menuitem = this.document.createElementNS(
"http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul",
"menuitem"
);
menuitem.dataset.command = command;
#populateResultMenu(
menupopup = this.resultMenu,
commands = this.#getResultMenuCommands(this.#resultMenuResult)
) {
menupopup.textContent = "";
for (let data of commands) {
if (data.children) {
let popup = this.document.createXULElement("menupopup");
this.#populateResultMenu(popup, data.children);
let menu = this.document.createXULElement("menu");
this.#setElementL10n(menu, data.l10n);
menu.appendChild(popup);
menupopup.appendChild(menu);
continue;
}
if (data.name == "separator") {
menupopup.appendChild(this.document.createXULElement("menuseparator"));
continue;
}
let menuitem = this.document.createXULElement("menuitem");
menuitem.dataset.command = data.name;
menuitem.classList.add("urlbarView-result-menuitem");
this.#setElementL10n(menuitem, data.l10n);
this.resultMenu.appendChild(menuitem);
menupopup.appendChild(menuitem);
}
}
@ -3008,7 +3034,7 @@ export class UrlbarView {
}
on_popupshowing(event) {
if (event.currentTarget == this.resultMenu) {
if (event.target == this.resultMenu) {
this.#populateResultMenu();
}
}

View File

@ -68,6 +68,17 @@ firefox-suggest-weather-high-low = High: { $high }°{ $unit } · Low: { $low }°
# $provider (String) - The name of the weather provider
firefox-suggest-weather-sponsored = { $provider } · Sponsored
firefox-suggest-weather-command-inaccurate-location =
.label = Report inaccurate location
firefox-suggest-weather-command-show-less-frequently =
.label = Show less frequently
firefox-suggest-weather-command-dont-show-this =
.label = Dont show this
firefox-suggest-weather-command-not-relevant =
.label = Not relevant
firefox-suggest-weather-command-not-interested =
.label = Not interested
## These strings are used in the preferences UI (about:preferences). Their names
## follow the naming conventions of other strings used in the preferences UI.

View File

@ -839,6 +839,10 @@ The event's objects are the following possible values:
The user picked the suggestion's help button.
:impression_only:
The user picked some other row.
:other:
The user engaged with the suggestion in some other way, for example by picking
a command in the result menu. This is a catch-all category and going forward
Glean telemetry should be preferred.
The event's ``extra`` contains the following properties:
@ -864,9 +868,13 @@ Changelog
Firefox 112.0
``navigational`` is added as a value of ``suggestion_type``. [Bug 1819797_]
Firefox 114.0
``other`` is added as a value of the event object. [Bug 1827943_]
.. _1761059: https://bugzilla.mozilla.org/show_bug.cgi?id=1761059
.. _1800993: https://bugzilla.mozilla.org/show_bug.cgi?id=1800993
.. _1819797: https://bugzilla.mozilla.org/show_bug.cgi?id=1819797
.. _1827943: https://bugzilla.mozilla.org/show_bug.cgi?id=1827943
contextservices.quicksuggest.impression_cap
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

View File

@ -257,8 +257,17 @@ urlbar:
engagement_type:
description: >
Records how the user selected the result. The possible values are:
`click`, `enter`, `go_button`, `drop_go`, `paste_go`, `dismiss`,
`help`.
`click`,
`dismiss`,
`drop_go`,
`enter`,
`go_button`,
`help`,
`inaccurate_location`,
`not_interested`,
`not_relevant`,
`paste_go`,
`show_less_frequently`
type: string
groups:
description: >

View File

@ -3633,7 +3633,10 @@ contextservices.quicksuggest:
"click": The user picked the suggestion.
"help": The user picked the suggestion's help button.
"impression_only": The user picked some other row.
objects: ["block", "click", "help", "impression_only"]
"other": The user engaged with the suggestion in some other way, for
example by picking a command in the result menu. This is a catch-all
category and going forward Glean telemetry should be preferred.
objects: ["block", "click", "help", "impression_only", "other"]
extra_keys:
match_type: >
"best-match" if the suggestion was a best match or "firefox-suggest" if