Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: EPNS embed sidebar notifications on Home page #1494

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

subhranshudas
Copy link

@subhranshudas subhranshudas commented May 9, 2022

Integration of EPNS Notifications Embed Feature on the Home Page

Description

On the Home page of the ENS app, when the user is connected to their account, we are basically showing an EPNS bell icon on click of which they will see a sidebar which has a list of notifications if they have "opted-in" (subscribed) to ENS channel via EPNS.

List of features added/changed

UX wise

  • An EPNS bell icon on the home page left of "My Account".
  • Click on the Bell Icon opens a sidebar with Notifications if opted in. (Most probably you may or may not see some test notifications for demo purpose only)

Code wise

  1. An EPNS Bell icon component which can be changed/customized as per need.
  2. An useEPNSEmbed hook which is used in the Home route wrapper and abstracts out the trigger & cleanup mechanism of the original EmbedSDK methods from the EPNS SDK. Feel free to change the init options here.
    {
        targetID: 'epns-trigger-id', // MANDATORY
        appName: 'ens', // MANDATORY
        user: '0xaddress1', // MANDATORY
        headerText: 'EPNS Notifications',
        viewOptions: {
            type: 'sidebar',
            showUnreadIndicator: true,
            unreadIndicatorColor: '#cc1919',
            unreadIndicatorPosition: 'top-right'
        }
    }
  1. A embedsdk.esm.js file (for now) to provide the EmbedSDK features from the original EPNS-SDK package.

Note: Currently we are transitioning to smaller individual packages, to avoid increasing the bundle size for ENS (or any dApp), we are now giving a build file which is going to be replaced by an NPM package shortly. So once that happens we can remove the build file and simply use in the useEPNSEmbed hook something like -

  import EmbedSDK from '@epnsproject/sdk-embed'

How Has This Been Tested?

Locally

  1. Ran the app in local with all the changes in Ropsten network with ENS sample account to land to the Home page with "My Account" displayed.
  2. The EPNS bell icon shows up only when "My Account" is displayed.
  3. Clicking on the Bell icon will show a sidebar which will have notifications if the user has subscribed to the ENS channel on EPNS.
  4. If the user is not connected to their account, the Bell Icon will not show.

Unit Tests

  1. Created/Updated the following tests which are passing -

    • Home.spec.js
    • useEPNSEmbed.spec.js
  2. Ran npm test on the entire code base -

    Test Suites: 26 passed, 26 total
    Tests:       1 todo, 140 passed, 141 total
    Snapshots:   0 total
    Time:        15.012 s
    

Checklist:

  • My code follows the code style of this project.
  • My code implements all the required features.

Screenshots

Screen Shot 2022-05-09 at 9 56 40 AM

Screen Shot 2022-05-09 at 9 57 14 AM

Screen Shot 2022-05-09 at 9 57 30 AM

Screen Shot 2022-05-09 at 9 58 27 AM

Screen Shot 2022-05-09 at 9 58 52 AM

@vercel
Copy link

vercel bot commented May 9, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
ens-app ✅ Ready (Inspect) Visit Preview May 23, 2022 at 6:27AM (UTC)

Copy link
Member

@LeonmanRolls LeonmanRolls left a comment

Choose a reason for hiding this comment

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

Hey @subhranshudas thanks very much for your PR! Some points/questions:

  • You mentioned you tested on ropsten however it seems EPNS does not support ropsten, and also ENS doesn’t support Kovan (d’oh!), is there a way for me to test what the notification will look like if I have a name that’s soon expiring without registering one on mainnet?
  • The EPNS plugin asks me to change to Kovan network intermittently, this should not happen.
  • The sidebar should not be showing demo content when there are no notifications. It can perhaps link to a tutorial if the user has not opted in to the ENS channel yet.

@subhranshudas
Copy link
Author

Hey @subhranshudas thanks very much for your PR! Some points/questions:

  • You mentioned you tested on ropsten however it seems EPNS does not support ropsten, and also ENS doesn’t support Kovan (d’oh!), is there a way for me to test what the notification will look like if I have a name that’s soon expiring without registering one on mainnet?
  • The EPNS plugin asks me to change to Kovan network intermittently, this should not happen.
  • The sidebar should not be showing demo content when there are no notifications. It can perhaps link to a tutorial if the user has not opted in to the ENS channel yet.

Hello @LeonmanRolls,
Thank you for your feedback!
1 & 3). Yes! we basically showed some demo notifications in the sidebar for Ropsten just to demo the UX of the sidebar so that it wouldn't look weird having a blank page! But we have (a) removed the demo content and now if you don't have any notifications you will see the attached view with a URL link for live walkthrough that explains how to use EPNS to opt in for notifications. (b) hiding the bell icon if on an EPNS un-supported network (taking a cue from #1497 )
Also attaching the ENS expiry notification from the ENS channel.

  1. I have tried to reproduce this, was not able to reproduce it on the deployed/local app. Would be great to get get some pointers on this to reproduce so as to debug.

Screen Shot 2022-05-23 at 8 51 07 PM
Screen Shot 2022-05-23 at 8 56 32 PM

@LeonmanRolls
Copy link
Member

Thanks @subhranshudas. Do you have a way to test the functionality with a name that is soon to expire without registering a new name on mainnet?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants