Add announcement area for new features #1561

Vinnl · 2022-02-21T15:54:52Z

New feature description

This adds a menu that highlights new features. Note that this is slightly limited compared to the acceptance criteria: since only a few entries are shown at this time, the menu shows the entire history. I've prepared a separate PR to implement the full feature set in React, which will be used by the time there are more entries: #1562.

Screenshot (if applicable)

Kooha-02-21-2022-16-39-31.mp4

How to test

Sign in with a pro account and with the add-on enabled.
See that there's a "News" entry in the menu, with a counter set to 3.
Verify that the counter is decremented when opening one of the entries for the first time.
With different accounts, verify that the counter is reset again. Additionally:
With the extension not installed, verify that there is no entry for signing back in.
Without Premium, verify that there is no entry about critical emails.

Checklist

l10n dependencies have been merged, if any. - Not yet: Week of Feb 21 PR mozilla-l10n/fx-private-relay-l10n#63
All acceptance criteria are met. - No, see above.
I've added or updated relevant docs in the docs/ directory.
All UI revisions follow the coding standards, and use Protocol tokens where applicable (see /static/scss/libs/protocol/css/includes/tokens/dist/index.scss).
Commits in this PR are minimal and have descriptive commit messages.

maxxcrawford

This PR isn't reviewable as there's no companion PR in the L10N repo to check against, so all the strings are ??? (However, it looks like the React PR has all the strings in the pendingTranslations.ftl file.

Added two CSS notes!

maxxcrawford · 2022-02-23T21:13:43Z

static/scss/components/whatsnew.scss

+                display: initial;
+            }
+
+            button {


Suggestion: this should be broken out into its own class/section: .c-whatsnew-entry-content

maxxcrawford · 2022-02-23T21:15:06Z

static/scss/components/whatsnew.scss

+                &:hover {
+                    background-color: $color-blue-50;
+                    color: $color-white;
+                }


issue (blocking): The hover state for each news item only the color of the description, but not the title (h3). They both should be updated to the same color.

Nice catch: bb9fa6c

But then Eduardo pointed out that the hover colour should be different, and the text colour is black again, so I removed that again :P e0a201d

Vinnl · 2022-02-24T10:45:49Z

l10n PR wasn't submitted yet (the strings in the React PR were tentative) because the content wasn't final yet, but it has now been submitted! mozilla-l10n/fx-private-relay-l10n#63

maxxcrawford

I did a round of just running/testing and did some quick design comparisons with the Figma layout. There are a a few major things missing, LMK if any of these were omitted intentionally (since we have the new front-end coming soon).

UX:

History/News toggle state
Close/cancel button in top right corner
Clear all button in bottom right corner

UI Differences:

Border in between two news items (and in the history/news toggle header)
Padding for elements inside the feed panel
- Note: It's 13px/6px for non-hovered/hovered states, so I would put a $spacing-sm on both the parent and child elements in the feed.
The hover color is wrong (but you're using the correct color ($color-violet-05). Can you confirm that the variables are actually overwriting the Protocol tokens?
Missing triangle bubble on top of news feed
The number (encased in yellow circle) is in italics
Additional horizontal padding on "News" item when on hover/active (at least left side, maybe both?)

Will do a deeper dive / code review tomorrow!

Vinnl · 2022-02-25T08:52:45Z

@maxxcrawford Yes, see my initial PR description :)

Note that this is slightly limited compared to the acceptance criteria: since only a few entries are shown at this time, the menu shows the entire history. I've prepared a separate PR to implement the full feature set in React, which will be used by the time there are more entries

Tony and Eduardo were OK with this approach. I've omitted the close button as well since there's no header now, and it can also be closed by clicking outside of the menu.

As for the remaining issues:

Border in between two news items

Padding for elements inside the feed panel

74c02b2

The hover color is wrong (but you're using the correct color ($color-violet-05). Can you confirm that the variables are actually overwriting the Protocol tokens?

So I investigated this, and it looks like

@import "libs/protocol/css/components/forms/choice.scss";

in filter-bar.scss again leads to an import of the Protocol tokens, which then overwrites the Nebula colours again. I could fix that for this specific colour by moving the filter-bar import below the whatsnew import (1947f47), but that does mean that much of our other styles don't actually seem to use the Nebula colours.

Removing that import from filter-bar broke a bunch of other things, so I propose just waiting for this to be fixed with the React frontend, where styles don't easily leak into other stylesheets.

Missing triangle bubble on top of news feed

ea2859d

The number (encased in yellow circle) is in italics

c500a52

Additional horizontal padding on "News" item when on hover/active (at least left side, maybe both?)

I tried fixing this, but the horizontal padding on the other menu items is a lot smaller, so if I were to make space for this, the "News" entry would look very out-of-place. I'm also a bit afraid to touch this, since the menu items are already a bit inconsistent between the Premium and Free themes, and so I'm not sure what I'd break if I were to start messing with those spacings. So I propose accepting the smaller padding for now.

Kooha-02-25-2022-09-53-41.mp4

codemist

A couple things, I can't seem to close the tab when clicking the news button again.
https://user-images.githubusercontent.com/13066134/156044262-c05fdbda-f307-4420-ae94-cd3ba2f1d379.mov

Another thing is that when I shrink the device width, the panel gets cut off at a certain breakpoint (around 762px). Are we keeping the whats new feature on mobile as well?
https://user-images.githubusercontent.com/13066134/156044637-352b3131-0126-4fa5-82e1-38f8a1748bb3.mov

maxxcrawford

Nice fixups. I had a ton of nits, and I appreciate you knocking them out.

I found one non-blocking CSS issue. I'm approving, but please be sure to check out @codemist's comments above. Thanks!

maxxcrawford · 2022-02-28T22:32:13Z

static/scss/components/whatsnew.scss

+        li {
+            display: none;
+            padding: $spacing-sm 0;
+            border-bottom: 1px solid $color-light-gray-30;


You'll want to add an extra rule to remove this for the last item.

I made the border color red to show its visibility:

maxxcrawford · 2022-03-01T00:14:27Z

Quick note: We're at "code freeze" today, so I'm going to merge this (after I fix the analytics conflict) and let you do a follow up PR for the other issues (including what I called out").

…ures

…ew features

…a for new features

Vinnl requested review from maxxcrawford and codemist February 21, 2022 15:54

Vinnl self-assigned this Feb 21, 2022

Vinnl mentioned this pull request Feb 21, 2022

Add announcement area for new features (React) #1562

Merged

5 tasks

Vinnl force-pushed the new-stuff branch from e255575 to 41738e6 Compare February 21, 2022 16:09

maxxcrawford suggested changes Feb 23, 2022

View reviewed changes

Vinnl requested a review from maxxcrawford February 24, 2022 10:46

Vinnl mentioned this pull request Feb 24, 2022

Add "tips" card at the bottom of the dashboard #1491

Closed

4 tasks

maxxcrawford reviewed Feb 24, 2022

View reviewed changes

maxxcrawford self-requested a review February 25, 2022 15:33

codemist reviewed Feb 28, 2022

View reviewed changes

maxxcrawford approved these changes Feb 28, 2022

View reviewed changes

Vinnl added 8 commits February 28, 2022 18:19

Add announcement area for new features

6010f03

fixup! Add announcement area for new features

2717cec

fixup! fixup! Add announcement area for new features

5ae1f07

fixup! fixup! fixup! Add announcement area for new features

71e4406

fixup! fixup! fixup! fixup! Add announcement area for new features

955bafa

fixup! fixup! fixup! fixup! fixup! Add announcement area for new feat…

9b76bcc

…ures

fixup! fixup! fixup! fixup! fixup! fixup! Add announcement area for n…

c563e28

…ew features

fixup! fixup! fixup! fixup! fixup! fixup! fixup! Add announcement are…

d33862e

…a for new features

maxxcrawford force-pushed the new-stuff branch from 1947f47 to d33862e Compare March 1, 2022 00:19

maxxcrawford merged commit f497a87 into main Mar 1, 2022

Vinnl deleted the new-stuff branch March 1, 2022 13:26

Vinnl mentioned this pull request Mar 1, 2022

Refinement for the New Feature Announcement area #1579

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add announcement area for new features #1561

Add announcement area for new features #1561

Vinnl commented Feb 21, 2022 •

edited

Loading

maxxcrawford left a comment

maxxcrawford Feb 23, 2022

Vinnl Feb 24, 2022

maxxcrawford Feb 23, 2022

Vinnl Feb 24, 2022

Vinnl commented Feb 24, 2022

maxxcrawford left a comment

Vinnl commented Feb 25, 2022 •

edited

Loading

codemist left a comment

maxxcrawford left a comment

maxxcrawford Feb 28, 2022

maxxcrawford commented Mar 1, 2022

Add announcement area for new features #1561

Add announcement area for new features #1561

Conversation

Vinnl commented Feb 21, 2022 • edited Loading

New feature description

Screenshot (if applicable)

How to test

Checklist

maxxcrawford left a comment

Choose a reason for hiding this comment

maxxcrawford Feb 23, 2022

Choose a reason for hiding this comment

Vinnl Feb 24, 2022

Choose a reason for hiding this comment

maxxcrawford Feb 23, 2022

Choose a reason for hiding this comment

Vinnl Feb 24, 2022

Choose a reason for hiding this comment

Vinnl commented Feb 24, 2022

maxxcrawford left a comment

Choose a reason for hiding this comment

Vinnl commented Feb 25, 2022 • edited Loading

codemist left a comment

Choose a reason for hiding this comment

maxxcrawford left a comment

Choose a reason for hiding this comment

maxxcrawford Feb 28, 2022

Choose a reason for hiding this comment

maxxcrawford commented Mar 1, 2022

Vinnl commented Feb 21, 2022 •

edited

Loading

Vinnl commented Feb 25, 2022 •

edited

Loading