Skip to content

Feature/transfer/withdrawal/profile - #3

Open
gabito1451 wants to merge 3 commits into
masterfrom
feature/Transfer/Withdrawal/Profile
Open

Feature/transfer/withdrawal/profile#3
gabito1451 wants to merge 3 commits into
masterfrom
feature/Transfer/Withdrawal/Profile

Conversation

@gabito1451

Copy link
Copy Markdown
Owner

Added Transfer, Withdrawal, Profile Page

<div>
<button
className="btn btn-primary text-xl mr-6 cursor-pointer"
e

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why did you add this?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I got this error on the console. You should always have the console open when working with Javascript so you can catch all errors/warnings and fix them.
Screenshot at Apr 19 10-15-56 AM

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Resolved

const currentUser = getUserByAccountNumber(currentUserAccountNumber);

const schema = yup.object({
selectAccount: yup.string(),

@nwankwo-ikemefuna nwankwo-ikemefuna Apr 19, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A more appropriate name for this field would be selectedAccount, which is a noun. selectAccount sounds like an action (a verb), which is not appropriate for input names (and any entity by extension). All input names must be semantically a noun.

Secondly, yup.string() alone is not enough validation rule for this input (an empty string "" will pass this validation). It should have a required validation rule too. As it is, if a user is not selected, the form can still be submitted, which is not desirable.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Resolved

export const Transfer = () => {
const registeredUsers = getAllUsers();
const navigate = useNavigate();
const [inputValue, setInputValue] = useState();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is not needed. useForm hook is registering and handling the input changes.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Resolved

.length(4, "Account PIN must be exactly 4 digits"),
});

const handleInputChange = (event) => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is not needed. useForm hook is registering and handling the input changes.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Resolved

type="password"
maxLength={4}
className="form-control"
onChange={handleInputChange}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is not needed. useForm hook is registering and handling the input changes.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Resolved

.length(4, "Account PIN must be exactly 4 digits"),
});

const handleInputChange = (event) => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is not needed. useForm hook is registering and handling the input changes.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Resolved

onChange={handleInputChange}
{...register("accountPin")}
/>
{inputValue}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is not needed. useForm hook is registering and handling the input changes.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Resolved

maxLength={4}
className="form-control"
required
onChange={handleInputChange}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is not needed. useForm hook is registering and handling the input changes.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Resolved

Comment thread src/styles/helper.css

/** Spacing */
.min-w-100 {
* Spacing */ .min-w-100 {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is an invalid css comment. Everything after this line will not be processed.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Resolved

return user?.transactions || [];
}
};
export const getUserAccountName = (

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This function adds unnecessary granularity to your code. You can get any user data from the user object returned by getUserByAccountNumber().

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Done

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.

3 participants