Here's the [Figma link](https://www.figma.com/file/JhKhHqJJ9bjzU1TITUpiWi/Review-Checker---Experiments?type=design&node-id=762-23740&mode=design&t=PPTe8L9lpC0glxgE-4) (screenshots to be added) 🚨 Warning - this bug probably should be split or revised. Ping :jhirsch before starting on it. ### Overview Up till now, closing the shopping sidebar (either by clicking the X in the header, or clicking the shopping icon in the urlbar) had a global effect: all shopping sidebars in the browser were closed, and we also set a pref so that the shopping sidebar would stay closed after restarting Firefox. The only way to get back to the shopping sidebar was for the user to manually reopen it by visiting a supported shopping page, then clicking the shopping icon in the urlbar. User research testing and quantitative feature retention data both suggest that this approach makes it too easy to 'lose' the feature, and too hard to get into the habit of using it. To improve rediscoverability of the feature, we now want to make the act of closing the shopping sidebar only hide the sidebar for the current tab, and only for the current page. If the user closes the sidebar in a given tab, then navigates to a new product page in the same tab, the sidebar will pop back up. In other bugs in this metabug, we will mitigate the annoyance factor by adding a new auto-open pref, and a new settings toggle to disable auto-open, as well as a card that offers to disable auto-open after the sidebar is dismissed 5 different times in one session. So, we're somewhat offsetting the annoyance factor by providing an option to make the feature really go away. In this bug, we are implementing the auto-open behavior. I think I've scouted out the main areas we'll need to adjust, but I may have missed some stuff. Ping me if you have questions or if this rough guide seems wrong (it probably is). ### 1. Replacing the 'active' pref with a list of tabs that have temporarily disabled the sidebar Currently, we toggle the visibility of the sidebar and urlbar icon by observing changes to the 'active' pref (technically, `browser.shopping.experience2023.active`). Instead, we will need to keep track of which tabs have had their sidebar hidden. Rather than add an expando property to individual browser elements, let's follow the approach sketched out in bug 1873780: create a WeakSet in ShoppingSidebarParent, named something like gClosedSidebarBrowsers (please, come up with a better name if you can!). When the user closes the sidebar on a given tab, add the browser's permanentKey to the WeakSet. When the tab navigates to a new shopping page, remove it from the WeakSet. Next, we'll want to update the UI code that toggles visibility of the sidebar, ShoppingSidebarParent._maybeToggleSidebar. Rather than checking the state of the 'active' pref to decide whether to show or hide UI, we'll instead want to check if gClosedSidebarBrowsers.has(browser.permanentKey). This brings us to the next loosely-defined subtask. ### 2. Changes to 'active' pref change listener functions Pref changes have a global pub-sub effect: changing a pref anywhere will trigger all the pref observer callbacks. We lose this functionality in moving from global to per-tab toggle behavior driven by the WeakSet described above. We'll need to look at the code path called when the active pref is changed (ShoppingSidebarParent.updateSidebarVisibility) and decide to do one of two things: either we keep global behavior, or switch to tab-local behavior. Global behavior to keep: closing all sidebars. We have code that hides all the sidebars, and we still want that code, except now, instead of being driven by the 'active' pref, it'll be driven by the 'autoOpen' pref which is being added in another bug (TODO document which bug). The pref will be `browser.shopping.experience2023.autoOpen`, and if it becomes false, we will want to close all the sidebars globally. Local behavior to add: closing just one sidebar. We'll need to change the click handler behavior from global to local. Instead of setting the 'active' pref to false when the user clicks the X, or toggling the 'active' pref value when the user clicks the shopping urlbar icon, we instead need those handlers to get the current browser, add its permanentKey to the WeakSet, and then hide the sidebar and set the shopping icon to the closed state (I guess by calling the _maybeUpdate functions, which would then need to be updated to check the WeakSet instead of checking the 'active' pref). Note that, in updating the _maybeToggle UI functions, you'll also want to account for the autoOpen pref, since the auto-open behavior can be disabled, but we don't want that to permanently disable the feature. Which brings us to the next task: ### 3. 'autoOpen' pref changes to UI toggle behavior If the user has disabled auto open (autoOpen pref set to false), and then visits a product page, _maybeToggleSidebar should not automatically show the sidebar, and _maybeToggleButton should show the button in its disabled/closed state. (Note that we have the `aIsNavigation` argument passed into _maybeToggleSidebar to help us determine which calls are a result of a navigation, and which calls are a result of a tab switch or pref flip.) However, if the user had disabled auto open in the past, and just clicked the shopping urlbar icon to redisplay the sidebar, we do want to show the sidebar UI and toggle the button state for that particular tab.
Bug 1876197 Comment 0 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
Here's the [Figma link](https://www.figma.com/file/JhKhHqJJ9bjzU1TITUpiWi/Review-Checker---Experiments?type=design&node-id=762-23740&mode=design&t=PPTe8L9lpC0glxgE-4) (screenshots to be added) 🚨 Warning - this bug probably should be split or revised. Ping :jhirsch before starting on it. If we retain the global toggle behavior, then the change here will just be something like: when the user closes the sidebar, start counting page views or minutes (TBD which we'll track), then when the count/timer runs out, if there's no callout displayed, flip the active pref. Note that we can check if a callout is displayed by importing FeatureCalloutBroker and checking isCalloutShowing. ----- original bug description: ### Overview Up till now, closing the shopping sidebar (either by clicking the X in the header, or clicking the shopping icon in the urlbar) had a global effect: all shopping sidebars in the browser were closed, and we also set a pref so that the shopping sidebar would stay closed after restarting Firefox. The only way to get back to the shopping sidebar was for the user to manually reopen it by visiting a supported shopping page, then clicking the shopping icon in the urlbar. User research testing and quantitative feature retention data both suggest that this approach makes it too easy to 'lose' the feature, and too hard to get into the habit of using it. To improve rediscoverability of the feature, we now want to make the act of closing the shopping sidebar only hide the sidebar for the current tab, and only for the current page. If the user closes the sidebar in a given tab, then navigates to a new product page in the same tab, the sidebar will pop back up. In other bugs in this metabug, we will mitigate the annoyance factor by adding a new auto-open pref, and a new settings toggle to disable auto-open, as well as a card that offers to disable auto-open after the sidebar is dismissed 5 different times in one session. So, we're somewhat offsetting the annoyance factor by providing an option to make the feature really go away. In this bug, we are implementing the auto-open behavior. I think I've scouted out the main areas we'll need to adjust, but I may have missed some stuff. Ping me if you have questions or if this rough guide seems wrong (it probably is). ### 1. Replacing the 'active' pref with a list of tabs that have temporarily disabled the sidebar Currently, we toggle the visibility of the sidebar and urlbar icon by observing changes to the 'active' pref (technically, `browser.shopping.experience2023.active`). Instead, we will need to keep track of which tabs have had their sidebar hidden. Rather than add an expando property to individual browser elements, let's follow the approach sketched out in bug 1873780: create a WeakSet in ShoppingSidebarParent, named something like gClosedSidebarBrowsers (please, come up with a better name if you can!). When the user closes the sidebar on a given tab, add the browser's permanentKey to the WeakSet. When the tab navigates to a new shopping page, remove it from the WeakSet. Next, we'll want to update the UI code that toggles visibility of the sidebar, ShoppingSidebarParent._maybeToggleSidebar. Rather than checking the state of the 'active' pref to decide whether to show or hide UI, we'll instead want to check if gClosedSidebarBrowsers.has(browser.permanentKey). This brings us to the next loosely-defined subtask. ### 2. Changes to 'active' pref change listener functions Pref changes have a global pub-sub effect: changing a pref anywhere will trigger all the pref observer callbacks. We lose this functionality in moving from global to per-tab toggle behavior driven by the WeakSet described above. We'll need to look at the code path called when the active pref is changed (ShoppingSidebarParent.updateSidebarVisibility) and decide to do one of two things: either we keep global behavior, or switch to tab-local behavior. Global behavior to keep: closing all sidebars. We have code that hides all the sidebars, and we still want that code, except now, instead of being driven by the 'active' pref, it'll be driven by the 'autoOpen' pref which is being added in another bug (TODO document which bug). The pref will be `browser.shopping.experience2023.autoOpen`, and if it becomes false, we will want to close all the sidebars globally. Local behavior to add: closing just one sidebar. We'll need to change the click handler behavior from global to local. Instead of setting the 'active' pref to false when the user clicks the X, or toggling the 'active' pref value when the user clicks the shopping urlbar icon, we instead need those handlers to get the current browser, add its permanentKey to the WeakSet, and then hide the sidebar and set the shopping icon to the closed state (I guess by calling the _maybeUpdate functions, which would then need to be updated to check the WeakSet instead of checking the 'active' pref). Note that, in updating the _maybeToggle UI functions, you'll also want to account for the autoOpen pref, since the auto-open behavior can be disabled, but we don't want that to permanently disable the feature. Which brings us to the next task: ### 3. 'autoOpen' pref changes to UI toggle behavior If the user has disabled auto open (autoOpen pref set to false), and then visits a product page, _maybeToggleSidebar should not automatically show the sidebar, and _maybeToggleButton should show the button in its disabled/closed state. (Note that we have the `aIsNavigation` argument passed into _maybeToggleSidebar to help us determine which calls are a result of a navigation, and which calls are a result of a tab switch or pref flip.) However, if the user had disabled auto open in the past, and just clicked the shopping urlbar icon to redisplay the sidebar, we do want to show the sidebar UI and toggle the button state for that particular tab.
Here's the [Figma link](https://www.figma.com/file/JhKhHqJJ9bjzU1TITUpiWi/Review-Checker---Experiments?type=design&node-id=762-23740&mode=design&t=PPTe8L9lpC0glxgE-4) (screenshots to be added) 🚨 Warning - this bug probably should be split or revised. Ping :jhirsch before starting on it. If we retain the global toggle behavior, then the change here will just be something like: when an opted-in user closes the sidebar, start counting page views or minutes (TBD which we'll track), then when the count/timer runs out, if there's no callout displayed, flip the active pref. Note that we can check if a callout is displayed by importing FeatureCalloutBroker and checking isCalloutShowing. ----- original bug description: ### Overview Up till now, closing the shopping sidebar (either by clicking the X in the header, or clicking the shopping icon in the urlbar) had a global effect: all shopping sidebars in the browser were closed, and we also set a pref so that the shopping sidebar would stay closed after restarting Firefox. The only way to get back to the shopping sidebar was for the user to manually reopen it by visiting a supported shopping page, then clicking the shopping icon in the urlbar. User research testing and quantitative feature retention data both suggest that this approach makes it too easy to 'lose' the feature, and too hard to get into the habit of using it. To improve rediscoverability of the feature, we now want to make the act of closing the shopping sidebar only hide the sidebar for the current tab, and only for the current page. If the user closes the sidebar in a given tab, then navigates to a new product page in the same tab, the sidebar will pop back up. In other bugs in this metabug, we will mitigate the annoyance factor by adding a new auto-open pref, and a new settings toggle to disable auto-open, as well as a card that offers to disable auto-open after the sidebar is dismissed 5 different times in one session. So, we're somewhat offsetting the annoyance factor by providing an option to make the feature really go away. In this bug, we are implementing the auto-open behavior. I think I've scouted out the main areas we'll need to adjust, but I may have missed some stuff. Ping me if you have questions or if this rough guide seems wrong (it probably is). ### 1. Replacing the 'active' pref with a list of tabs that have temporarily disabled the sidebar Currently, we toggle the visibility of the sidebar and urlbar icon by observing changes to the 'active' pref (technically, `browser.shopping.experience2023.active`). Instead, we will need to keep track of which tabs have had their sidebar hidden. Rather than add an expando property to individual browser elements, let's follow the approach sketched out in bug 1873780: create a WeakSet in ShoppingSidebarParent, named something like gClosedSidebarBrowsers (please, come up with a better name if you can!). When the user closes the sidebar on a given tab, add the browser's permanentKey to the WeakSet. When the tab navigates to a new shopping page, remove it from the WeakSet. Next, we'll want to update the UI code that toggles visibility of the sidebar, ShoppingSidebarParent._maybeToggleSidebar. Rather than checking the state of the 'active' pref to decide whether to show or hide UI, we'll instead want to check if gClosedSidebarBrowsers.has(browser.permanentKey). This brings us to the next loosely-defined subtask. ### 2. Changes to 'active' pref change listener functions Pref changes have a global pub-sub effect: changing a pref anywhere will trigger all the pref observer callbacks. We lose this functionality in moving from global to per-tab toggle behavior driven by the WeakSet described above. We'll need to look at the code path called when the active pref is changed (ShoppingSidebarParent.updateSidebarVisibility) and decide to do one of two things: either we keep global behavior, or switch to tab-local behavior. Global behavior to keep: closing all sidebars. We have code that hides all the sidebars, and we still want that code, except now, instead of being driven by the 'active' pref, it'll be driven by the 'autoOpen' pref which is being added in another bug (TODO document which bug). The pref will be `browser.shopping.experience2023.autoOpen`, and if it becomes false, we will want to close all the sidebars globally. Local behavior to add: closing just one sidebar. We'll need to change the click handler behavior from global to local. Instead of setting the 'active' pref to false when the user clicks the X, or toggling the 'active' pref value when the user clicks the shopping urlbar icon, we instead need those handlers to get the current browser, add its permanentKey to the WeakSet, and then hide the sidebar and set the shopping icon to the closed state (I guess by calling the _maybeUpdate functions, which would then need to be updated to check the WeakSet instead of checking the 'active' pref). Note that, in updating the _maybeToggle UI functions, you'll also want to account for the autoOpen pref, since the auto-open behavior can be disabled, but we don't want that to permanently disable the feature. Which brings us to the next task: ### 3. 'autoOpen' pref changes to UI toggle behavior If the user has disabled auto open (autoOpen pref set to false), and then visits a product page, _maybeToggleSidebar should not automatically show the sidebar, and _maybeToggleButton should show the button in its disabled/closed state. (Note that we have the `aIsNavigation` argument passed into _maybeToggleSidebar to help us determine which calls are a result of a navigation, and which calls are a result of a tab switch or pref flip.) However, if the user had disabled auto open in the past, and just clicked the shopping urlbar icon to redisplay the sidebar, we do want to show the sidebar UI and toggle the button state for that particular tab.
Here's the [Figma link](https://www.figma.com/file/JhKhHqJJ9bjzU1TITUpiWi/Review-Checker---Experiments?type=design&node-id=762-23740&mode=design&t=PPTe8L9lpC0glxgE-4) (screenshots to be added) Roughly: when an opted-in user closes the sidebar, start counting page views or minutes (TBD which we'll track), then when the count/timer runs out, if there's no callout displayed, flip the active pref. Note that we can check if a callout is displayed by importing FeatureCalloutBroker and checking isCalloutShowing. Since this change is adding an app-global counter, ShoppingUtils is probably a natural spot for it.
Here's the [Figma link](https://www.figma.com/file/JhKhHqJJ9bjzU1TITUpiWi/Review-Checker---Experiments?type=design&node-id=762-23740&mode=design&t=PPTe8L9lpC0glxgE-4) (screenshots to be added) Roughly: when an opted-in user closes the sidebar, flipping the `active` pref to false, if the `browser.shopping.experience2023.autoOpen` pref is true, start counting page views or minutes (TBD which we'll track), then when the count/timer runs out, if there's no callout displayed, flip the `active` pref back to true. Note that we can check if a callout is displayed by importing FeatureCalloutBroker and checking isCalloutShowing. Since this change is adding an app-global counter, ShoppingUtils is probably a natural spot for it. Still waiting on product to tell us the frequency. Hopefully will have more details today (24 Jan).
Here's the [Figma link](https://www.figma.com/file/JhKhHqJJ9bjzU1TITUpiWi/Review-Checker---Experiments?type=design&node-id=762-23740&mode=design&t=PPTe8L9lpC0glxgE-4) (screenshots to be added) Roughly: when an opted-in user closes the sidebar, flipping the `active` pref to false, if the `browser.shopping.experience2023.autoOpen` pref is true, start counting page views or minutes (TBD which we'll track), then when the count/timer runs out, if there's no callout displayed, flip the `active` pref back to true. Note that we can check if a callout is displayed by importing FeatureCalloutBroker and checking isCalloutShowing. Since this change is adding an app-global counter, ShoppingUtils is probably a natural spot for it. We've now heard that the frequency is supposed to mimic auto-open behavior. So, you close the sidebar, navigate to another product page, and the sidebar comes back. There is no delay except waiting for the user to navigate to a new product page.