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

Email App Revamp #31

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

Email App Revamp #31

wants to merge 5 commits into from

Conversation

riyanshu-patro
Copy link
Contributor

@riyanshu-patro riyanshu-patro commented Dec 23, 2024

Attached screenshots for the developed Email app -

Screenshot 2024-12-23 at 3 44 16 PM Screenshot 2024-12-23 at 3 47 03 PM Screenshot 2024-12-18 at 3 57 19 PM Screenshot 2024-12-18 at 3 57 56 PM Screenshot 2024-12-18 at 4 00 00 PM Screenshot 2024-12-18 at 3 59 52 PM Screenshot 2024-12-18 at 4 00 16 PM

Pending tasks -

  • Add description for landing page (pending from Zee)
  • Integrating pushchain-ui-kit (walletType is required for fetching emails and displaying wallet icons)
  • dummy email for new users (pending from Zee)
  • file attachment (design is yet to be reviewed by Zee)

@@ -1,34 +1,280 @@
import { usePrivy } from '@privy-io/react-auth';
import { useAppContext } from '@/context/app-context';
Copy link
Contributor

Choose a reason for hiding this comment

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

@riyanshu-patro the landing page UI is not correct and should match the exact designs. Please refer to the core-connection example app

usePushWalletContext,
} from '@pushprotocol/pushchain-ui-kit';

const Header: FC = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

@riyanshu-patro The header component should be a fixed component and should not scroll with the viewport when user tries to scroll down.

import { trimAddress, formatTimestamp } from '@/lib/utils';
import { PushNetwork } from '@pushprotocol/push-chain';
import {
Copy link
Contributor

Choose a reason for hiding this comment

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

@riyanshu-patro the attachment input is broken and I can't attach a file

@@ -2,6 +2,8 @@ import EmailCard from './email-card';
import { ScrollArea } from './ui/scroll-area';
import { useAppContext } from '@/context/app-context';
import { EMAIL_BOX } from '@/constants';
import { Box } from 'shared-components';
import { dummyEmail } from '@/lib/utils';
Copy link
Contributor

Choose a reason for hiding this comment

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

Screenshot 2024-12-23 at 4 15 25 PM

From and to are same for a email I replied to.

@@ -2,6 +2,8 @@ import EmailCard from './email-card';
import { ScrollArea } from './ui/scroll-area';
import { useAppContext } from '@/context/app-context';
import { EMAIL_BOX } from '@/constants';
import { Box } from 'shared-components';
import { dummyEmail } from '@/lib/utils';
Copy link
Contributor

Choose a reason for hiding this comment

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

Screenshot 2024-12-23 at 4 15 59 PM

The to section is empty. Should show the wallet address.

const [isOpen, setIsOpen] = useState(false);

const { pushAccount, pushNetwork, setEmails } = useAppContext();
const { signMessageAsync } = useSignMessage();
const {
Copy link
Contributor

Choose a reason for hiding this comment

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

Validations also needs to be added to the To field and the message field.

SendNotification,
Text,
TextArea,
TextInput,
Copy link
Contributor

Choose a reason for hiding this comment

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

When user focus out of the to field it should automatically accept the entered text and the recipient.

import { trimAddress, formatTimestamp } from '@/lib/utils';
import { PushNetwork } from '@pushprotocol/push-chain';
import {
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to make a network call after sending an email to update the sent email list.

import { trimAddress, formatTimestamp } from '@/lib/utils';
import { PushNetwork } from '@pushprotocol/push-chain';
import {
Box,
Copy link
Contributor

Choose a reason for hiding this comment

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

Make sure the email listing and search doesn't breaks with a large number of emails in the response.

@@ -0,0 +1,144 @@
import { Back, Box, PushLogo, Text } from 'shared-components';
import { Card } from './ui/card';
import { css } from 'styled-components';
Copy link
Contributor

Choose a reason for hiding this comment

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

  • File names should be Capitalised camel case.
  • Folder structure also needs to be updated.
    Please refer to the core-connection repo for that.

<Box display="flex" alignItems="center" gap="spacing-xs">
<Box display="flex" alignItems="center" gap="spacing-xxs">
<PushLogo height={40} />
<Text variant="h2-bold">Push</Text>
Copy link
Contributor

Choose a reason for hiding this comment

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

Screenshot 2024-12-23 at 4 53 48 PM

Maybe remove the Push text for now in the mobile view,=,

Comment on lines 31 to 35
display: block;
@media (max-width: 768px) {
display: none;
}
`}
Copy link
Contributor

Choose a reason for hiding this comment

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

not the correct way. Wrap text inside the box component and use the box display property to implement it.

@@ -0,0 +1,45 @@
import { FC } from 'react';
import { Box, PushLogo, Text } from 'shared-components';
import { css } from 'styled-components';
Copy link
Contributor

Choose a reason for hiding this comment

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

@riyanshu-patro you could uninstall the styled components from here and use the one from shared-components.

@@ -2,15 +2,9 @@ import { StrictMode } from 'react';
import { createRoot } from 'react-dom/client';
import App from './App.tsx';
import './index.css';
import { AppProvider } from './providers/app-provider.tsx';
import PrivyWalletProvider from './providers/privy-wallet-provider.tsx';
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can delete the privy provider file.

@@ -1,31 +1,31 @@
import * as React from "react"
import * as PopoverPrimitive from "@radix-ui/react-popover"
import * as React from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think this file is required

};

return (
<div className="border border-gray-300 rounded-md flex items-center px-3 py-2">
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we using tailwind css classes here. That needs to be uninstalled and we need to use the box component

className
)}
{...props}
>
<ScrollAreaPrimitive.ScrollAreaThumb className="relative flex-1 rounded-full bg-border" />
</ScrollAreaPrimitive.ScrollAreaScrollbar>
))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we not using select from design system?

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