Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

[EPIC] Feature discoverability - Beamer #3515

Merged
merged 9 commits into from
Mar 3, 2022
Merged

Conversation

DiogoSoaress
Copy link
Member

@DiogoSoaress DiogoSoaress commented Feb 15, 2022

What it solves

Part of Problem Statement safe-global/safe-pm#26
Resolves #3413

Related tasks

How this PR fixes it

Integrates Beamer as a 3rd party solution

How to test it

  • Opting on/off in the "Community support & updates" cookies enables/disables Beamer loading as a sliding section for feature discoverability and announcements.

Analytics changes

Screenshots

Screen Shot 2022-02-17 at 11 04 00

Screen Shot 2022-02-17 at 11 12 43

* feat: Add whats new button in sidebar, include beamer script

* Use src branch dependency and continue customizing beamer

* style: notifications badge

* Load Beamer from the CookieBanner preferences

* update beamer config object on close

* chore: Bump version on components package

* chore: Add beamer type to global window type and remove ts-nocheck

* fix: useRef for beamer script tag to remove

* fix: Add Beamer params and methods type definitions

* refactor: Pass generic to loadFromCookie

* fix: pass value as Generic instead of the whole Record<string, T> type

* fix: PR feedback

Co-authored-by: Usame Algan <usame.algan@gnosis.pm>
@github-actions
Copy link

CLA Assistant Lite All Contributors have signed the CLA.

@github-actions
Copy link

github-actions bot commented Feb 15, 2022

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@github-actions
Copy link

Deployment links

🟠 Rinkeby Mainnet 🟣 Polygon 🟡 BSC Arbitrum 🟢 Gnosis Chain

@coveralls
Copy link

coveralls commented Feb 15, 2022

Pull Request Test Coverage Report for Build 1928659583

  • 18 of 132 (13.64%) changed or added relevant lines in 15 files are covered.
  • 16 unchanged lines in 5 files lost coverage.
  • Overall coverage decreased (-0.5%) to 34.89%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/logic/cookies/store/selectors/index.ts 0 1 0.0%
src/components/List/index.tsx 0 2 0.0%
src/logic/cookies/store/reducer/cookies.ts 1 3 33.33%
src/logic/cookies/utils/index.ts 4 7 57.14%
src/utils/googleAnalytics.ts 4 7 57.14%
src/utils/storage/Storage.ts 1 4 25.0%
src/utils/intercom.ts 0 5 0.0%
src/components/AppLayout/Sidebar/index.tsx 0 10 0.0%
src/utils/beamer.ts 0 27 0.0%
src/components/CookiesBanner/index.tsx 0 58 0.0%
Files with Coverage Reduction New Missed Lines %
src/components/AppLayout/Footer/index.tsx 1 90.91%
src/components/AppLayout/Sidebar/index.tsx 3 0%
src/components/CookiesBanner/index.tsx 3 0%
src/utils/intercom.ts 3 0%
src/logic/hooks/useSafeAppUrl.tsx 6 0%
Totals Coverage Status
Change from base Build 1927052380: -0.5%
Covered Lines: 3366
Relevant Lines: 8719

💛 - Coveralls

@github-actions
Copy link

github-actions bot commented Feb 15, 2022

E2E Tests Failed
Check the results here: https://github.com/gnosis/safe-react-e2e-tests/actions/runs/1928705166

Failed tests:

  • ❌ Read-only transaction creation and review Read-only transaction creation and review
  • ❌ Safe Apps List Safe Apps List

DiogoSoaress and others added 2 commits February 17, 2022 10:14
* feat: link Beamer with the analytics cookies. Display different CookieBanner warning messages.

* refactor: extract to an object cookie warning messages

* refactor: clean up cookies reducer

Co-authored-by: Aaron Cook <iamacook@users.noreply.github.com>

* fix: update Beamer cookie alert copy

* fix: handle Beamer with update cookies

Co-authored-by: Aaron Cook <iamacook@users.noreply.github.com>
Copy link
Member

@iamacook iamacook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loving the TypeScript 😉

It won't let me comment on it as it's legacy, but on line 258 in src/components/CookiesBanner/index.tsx you can simplify it to `onClick={closeCookiesBannerHandler}.

const handleClick = async () => {
const cookiesState = await loadFromCookie<BannerCookiesType>(COOKIES_KEY)
if (!cookiesState) {
dispatch(openCookieBanner({ cookieBannerOpen: true }))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible to simplify this action even more. If openCookieBanner is dispatched, you need not send { cookieBannerOpen: true } in the payload at all.

<StyledListItemText>What&apos;s new</StyledListItemText>
</StyledListItem>

<HelpCenterLink href="https://help.gnosis-safe.io/en/" target="_blank" title="Help Center of Gnosis Safe">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably out of scope, but have we considered treating links as CONSTANTS and/or centralising them? Would help in the future if need to update any. Maybe worth creating a new issue? cc @katspaugh

@@ -156,55 +158,57 @@ const CookiesBanner = (): ReactElement => {
}
}
fetchCookiesFromStorage()
}, [showAnalytics, showIntercom])
}, [dispatch, showAnalytics, showIntercom])

const acceptCookiesHandler = async () => {
const newState = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could add the types to these newState objects. Not necessary though.

@DiogoSoaress
Copy link
Member Author

The last commit aims to allow QA to test the production styles.

Will be reverted after testing.

* fix: Remove Beamer cookies on consent withdraw, extract removeCookies function

* refactor: Rename domain variable
@katspaugh katspaugh temporarily deployed to Manual March 3, 2022 09:15 Inactive
@github-actions
Copy link

github-actions bot commented Mar 3, 2022

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 2 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@katspaugh katspaugh temporarily deployed to Manual March 3, 2022 09:25 Inactive
@usame-algan usame-algan temporarily deployed to Manual March 3, 2022 09:38 Inactive
@francovenica
Copy link
Contributor

I'll report this here since the 3610 was already merged, but this issue also happens in the 3610.

There is 1 beamer cookie that is not being removed when you withdraw consent on the settings preferences.
"_BEAMER_BOOSTED_ANNOUNCEMENT_DATE_ehlRMhQi41258"
image

Just check the checkbox in the preferences and reload the page to see all the cookies, Then uncheck the checkbox, reload the page and you will see that there is 1 cookie that is not going away

@iamacook
Copy link
Member

iamacook commented Mar 3, 2022

There is 1 beamer cookie that is not being removed when you withdraw consent on the settings preferences.

This should be covered by #3615.

* Refactor CookieBanner

* Hide What's New in desktop app

* Remove unrelated test from footer tests
@katspaugh katspaugh temporarily deployed to Manual March 3, 2022 14:48 Inactive
@francovenica
Copy link
Contributor

Yeah, the 3615 was merged and solves the issue with that cookie.
The "_BEAMER_BOOSTED_ANNOUNCEMENT_DATE_ehlRMhQi41258" is now removed.

Looks good

@katspaugh katspaugh merged commit 7e05fc3 into dev Mar 3, 2022
@katspaugh katspaugh deleted the feature-discoverability branch March 3, 2022 15:24
@github-actions github-actions bot locked and limited conversation to collaborators Mar 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[EPIC] Improving new feature discoverability
6 participants