-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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: add email blocklist section with mocked data #3145
feat: add email blocklist section with mocked data #3145
Conversation
@thaisguigon @charlesBochet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lakshay-saini-au8 Thank you very much for contributing. I left a couple of comments, but great work overall :)
const updateBlockedEmailList = (email: string) => | ||
setBlockedEmailList((prevState) => [ | ||
...prevState, | ||
{ id: v4(), email: email, blocked_at: '21/12/2023' }, | ||
]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Later we will need to check if the value email
is indeed an email and we will also need to check if the email isn't already blocked.
I think that even if these are mock data, we could put the current date in blocked_at
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we can put the check later on once we integrate with the backend, also I didn't find any error state design for this in Figma.
<StyledContainer> | ||
<StyledLinkContainer> | ||
<TextInput | ||
placeholder="[email protected]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the Figma, the placeholder should be "[email protected], @apple.com".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@bosiraphael Please review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you !
#2832
Untitled_Project_V1.mp4