[MPP-1708] Popup Refactor - Add-on QA Fixes #472

maxxcrawford · 2023-02-15T09:15:53Z

Summary

This PR addresses multiple QA issues found in initial testing from @AOiegas

Other fixes beyond what QA has filed:

Added logic to hide the navigation item for the News panel if there are no news items available to show (unread or otherwise)
- When testing this, you can use a VPN on Stage to limit which is visible. Note that the Firefox Integration flag is set to true on Stage, so it will always return at least one news item
Added logic to set focus state correctly everytime you open the panel (regardless of logged in/out state or tier/mask count)

Known caveats:

The "new item" count is not working currently

Links:

Jira Tickets:
- https://mozilla-hub.atlassian.net/browse/MPP-2857 (Loading Error when Installing)
- https://mozilla-hub.atlassian.net/browse/MPP-2848 (Settings while signed in error)
Epic PR [MPP-1708] Popup Refactor (Epic PR to Main) #456

Testing

MPP-2857

Dev Note: Do not use web-ext for this test

Open up clean install/profile of Firefox
Log into Relay
Close Relay dashboard page
Go to about:debugging or about:addons and install add-on
Open panel
Expected: You should get the login panel

MPP-2857

Install add-on
Do NOT log in
Open panel
Expected: You should get the login panel
Click on Settings navigation item
Expected: You should get the settings panel AND the "Report Issue" link should be disabled/faded out/unclickable
Click back button
Expected: You should get the login panel again

Screenshots

N/A

… for any news items

src/css/popup.css

src/js/popup/popup.js

codemist

Had a comment about news items, probably unblocking but just wanted to check with you if that use case is considered (also okay if those old news items are deprecated, since they aren't really news anymore).

Tested this on a fresh instance of a firefox profile, test cases works as expected.

codemist · 2023-02-16T15:22:06Z

src/js/popup/popup.js

+        // Remove news nav link if there's no news items to display to user
+        if (state.newsContent.length === 0 ) {
+          document.querySelector(".fx-relay-menu-dashboard-link[data-panel-id='news']").remove();
+          return;
+        }


qn: I recall the spec before was that the users would be showed old panels (25mb limit, block promotional emails, etc) if there are no "new" news items (phone, fx integration, bundle) to display, which is a state that users outside of US/CAN are particularly more inclined to see. Is that surfaced in the refactor?

No. I confirmed with @tcinotto that we aren't going to reuse previous news items.

The new UX here is that if there's no eligible news items (user cannot upgrade, buy phones, etc) then we just hide the news navigation/panel altogether.

* Move news annoucements for geo-specific items behind correct logic gates * WIP: Add additional logged in/out loggic, and add focus states * Add additional logic to focus on correct element * Move news items to state * Add logic to remove the news panel all together if user is ineligible for any news items * Refactor news list building logic inside the core popup function * Move Firefox Integration news item into logic check * Remove 'report issue' link if not logged in

@lloan

* [MPP-1714] Popup Refactor - Panel Navigation and Header Bar (#453) * Stub new work area for toolbar * Add new navigation icons * Add hard-coded (EN) labels for navigation * Start popup javascript logic (including init, panel switching, and login detection) * Remove commented out code * FOLD SVG name update * Add logic to handle external link clicks in popup, update HTML with data attrs to set URLs correctly with telemetry data * Add back button support for main panels * [MPP-1714] Popup Refactor - Settings Panel (#455) * Stub new work area for toolbar * Add new navigation icons * Add hard-coded (EN) labels for navigation * Start popup javascript logic (including init, panel switching, and login detection) * Remove commented out code * FOLD SVG name update * Add logic to handle external link clicks in popup, update HTML with data attrs to set URLs correctly with telemetry data * Add back button support for main panels * Create settings page content, style toggle buttons to match Nebula * Stub Issue Report page, add remaining Settings page external link * Add logic to handle child panel navigation * Fix issue where external links where not opening to correct URL * Remove global namespace * Revise spacer layout on settings/toggle wrapper * [MPP-1714] Popup Refactor - Report Issue Panel (#457) * Stub new work area for toolbar * Add new navigation icons * Add hard-coded (EN) labels for navigation * Start popup javascript logic (including init, panel switching, and login detection) * Remove commented out code * FOLD SVG name update * Add logic to handle external link clicks in popup, update HTML with data attrs to set URLs correctly with telemetry data * Add back button support for main panels * Create settings page content, style toggle buttons to match Nebula * Stub Issue Report page, add remaining Settings page external link * Add logic to handle child panel navigation * Fix issue where external links where not opening to correct URL * Move getBrowser function to shared utils * Optimize Report Issue success image * Add Report Issue panel content/logic from previous popup file * [MPP-1714] Popup Refactor - Stats Panel (#458) * Port stats from previous popup to new stats panel * Add second stats column, add new strings from L10N PR * Add logic to calculate mask stats used on the current website * Add additional logic to query custom masks for premium users * Remove commented code, and comment for magic number * Spelling fix * [MPP-1708] Popup Refactor - News Panel (#461) * Add news feature This feature has two panels: the news panel and the news story panel. - The news panel lists all news stories - The news story panel is the detail view when you click on a news item Both sections are built dynamically from the newsContent object. There are a few attributes you can add here to will limit who the news items is shown to (via waffle flags, etc) Optional data wise, you can also include (or not include) a CTA for the detailed view * Add Firefox Inegration panel * Add additional news content * Add swith/check to conditionally show/hide banners based on locale/subscribed status * WIP Number * Add UI and logic to show notification count for unread news items * Remove unlocalized strings * [MPP-1708] Popup Refactor - Masks Panel (#463) * New icons * WIP: Add mask UI/logic to display and create new masks for free users * Add copy to clipboard functionality * Add loading state This loading state is copied from the in-page content UI * Improve logic to show/hide elements based on mask count and account level * Add styles for premium masks to display past the main viewport * Add Mask Created label to new masks made from the panel * CSS Review comments * Add login screen, additional CSS refinements (#464) * [MPP-1708] Popup Refactor - Register Subdomain / Create Custom Masks (#466) * Add prompt for premium users to register subdomain if not yet set * Add new icons * Add revised Nebula error colors * Refactor how hostnames are requested from bg.js script * Add new bg function to create custom masks for premium users * Add PSL library for custom-mask prepopulate mask name feature * Add custom mask creation panel, including JS logic and styles * Add localized promo blocking tooltip strings * Review revisions: Migrate font-size to use global variables, remove comments, add aria role * [MPP-1708] Popup Refactor - Search Premium Masks (#467) * Add search feature for premium users * Add additional data (website usage/generated for) to filter by in panel * Add logic to handle premium users with no masks created * Remove mask expand/manage button (P2 item), update CSS to center copy button vertically in mask card list (#468) * Fix issue where masks do not work if not syncing labels * Remove all previous popup HTML/CSS/JS code * [MPP-1708] Popup Refactor - Add-on QA Fixes (#472) * Move news annoucements for geo-specific items behind correct logic gates * WIP: Add additional logged in/out loggic, and add focus states * Add additional logic to focus on correct element * Move news items to state * Add logic to remove the news panel all together if user is ineligible for any news items * Refactor news list building logic inside the core popup function * Move Firefox Integration news item into logic check * Remove 'report issue' link if not logged in * mpp 1714 staging qa fixes (#474) * Fix issue where news item would not open correctly * Fix issue where 'Mask created' label is covered up by mask cards * Fix issue where some masks were too long and breaking the display panel * (Fix #475) Review Fixes for Add-on Refactor (#477) * Fix #475 - Localize navigation items * Update anchor elements to buttons if they are internal navigation items, update styles * Add localized alt-tag info to navigation images (back button, close/dismiss) * Add comments for dynamically generated content within the HTML * Update all custom elements to contain a hyphen, rename news items variables to be more readable * Remove line-breaks, commented code * Update state object name to represent its context (now sessionState) * Save `isPremiumSubdomainSet` query to reduce localStorage calls * Update storyData to match other newsItem variable naming pattern * Remove console log, add code comments, move local storage function (TODO) into seperate function * Revert CSS attribute name, add CSS comments, update values to use vars * CSS fix for button HTML swap on Settings page * Remove logicCheck fields/logic as populateNewsFeed covers this functionality * Remove TODO * DRY Logic loop fix, add comments for mask creation error codes * Fix news system to show notification bug but store read count * Update premium links to open in correct anchored aarea * Fix alignment of checkboxes on web compat form * Add logic to only allow web compat submissions with necessary info * Refactor how totals are calculated for mask/forward/blocked counts * Updated strings * Fix return error code for makeDomainAddress * DRY logic fix from @lloan, better quailify bg function to get getCurrentPageHostname * Bump version number for next release

Move news annoucements for geo-specific items behind correct logic gates

65793cc

github-actions bot deployed to review February 15, 2023 09:15 Active

maxxcrawford changed the base branch from main to mpp-1714-staging February 15, 2023 09:16

WIP: Add additional logged in/out loggic, and add focus states

5b56e10

github-actions bot had a problem deploying to review February 15, 2023 18:05 Failure

Add additional logic to focus on correct element

9eb8867

github-actions bot had a problem deploying to review February 15, 2023 20:51 Failure

maxxcrawford marked this pull request as ready for review February 15, 2023 20:56

Move news items to state

197d0f7

github-actions bot deployed to review February 15, 2023 21:07 Active

Add logic to remove the news panel all together if user is ineligible…

99696d6

… for any news items

github-actions bot deployed to review February 15, 2023 21:16 Active

maxxcrawford changed the title ~~MPP 1717 addon popup fix geo issue~~ [MPP-1708] Popup Refactor - Add-on QA Fixes Feb 15, 2023

Refactor news list building logic inside the core popup function

9e13131

github-actions bot deployed to review February 15, 2023 21:34 Active

Move Firefox Integration news item into logic check

639ebfa

github-actions bot deployed to review February 15, 2023 21:43 Active

codemist reviewed Feb 16, 2023

View reviewed changes

src/css/popup.css Outdated Show resolved Hide resolved

codemist reviewed Feb 16, 2023

View reviewed changes

src/js/popup/popup.js Show resolved Hide resolved

codemist approved these changes Feb 16, 2023

View reviewed changes

Remove 'report issue' link if not logged in

6fa6f79

github-actions bot deployed to review February 16, 2023 15:27 Active

maxxcrawford merged commit 16f8878 into mpp-1714-staging Feb 16, 2023

maxxcrawford deleted the MPP-1717-addon-popup-fix-geo-issue branch February 16, 2023 15:32

maxxcrawford mentioned this pull request Feb 16, 2023

mpp 1714 staging qa fixes #474

Merged

maxxcrawford mentioned this pull request Feb 24, 2023

(Fix #475) Review Fixes for Add-on Refactor #477

Merged

32 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MPP-1708] Popup Refactor - Add-on QA Fixes #472

[MPP-1708] Popup Refactor - Add-on QA Fixes #472

maxxcrawford commented Feb 15, 2023 •

edited

Loading

codemist left a comment •

edited

Loading

codemist Feb 16, 2023 •

edited

Loading

maxxcrawford Feb 16, 2023

[MPP-1708] Popup Refactor - Add-on QA Fixes #472

[MPP-1708] Popup Refactor - Add-on QA Fixes #472

Conversation

maxxcrawford commented Feb 15, 2023 • edited Loading

Summary

Testing

Screenshots

codemist left a comment • edited Loading

Choose a reason for hiding this comment

codemist Feb 16, 2023 • edited Loading

Choose a reason for hiding this comment

maxxcrawford Feb 16, 2023

Choose a reason for hiding this comment

maxxcrawford commented Feb 15, 2023 •

edited

Loading

codemist left a comment •

edited

Loading

codemist Feb 16, 2023 •

edited

Loading