Skip to content

Added profile page #9

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.

Sign up for GitHub

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

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

Added profile page #9

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e642dc0
Added profile page
Gabbiechilz Feb 20, 2023
92cfad5
made the necessary corrections
Gabbiechilz Feb 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 94 additions & 0 deletions profile.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta http-equiv="X-UA-Compatible" content="IE=edge" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<link rel="stylesheet" href="./helpers/components.css" />
<link rel="stylesheet" href="./styles.css" />
<title>Profile</title>
<script src="./tailwind.js"></script>
</head>
<body>
<nav class="bg-gray-800 text-white p-6">
<ul class="flex justify-between items-center">
<li><a href="./transactions.html">Transactions</a></li>
<li><a href="./deposit.html">Deposit</a></li>
<li><a href="./withdraw.html">Withdraw</a></li>
<li><a href="./transfer.html">Transfer</a></li>
<li><a href="#">Account Profile</a></li>
<li><a id="logout-btn" href="#">Logout</a></li>
</ul>
</nav>
Copy link
Copy Markdown
Collaborator

@nwankwo-ikemefuna nwankwo-ikemefuna Feb 22, 2023

Choose a reason for hiding this comment

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

Where's the account balance section?

Copy link
Copy Markdown
Owner Author

@gabito1451 gabito1451 Feb 27, 2023

Choose a reason for hiding this comment

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

I've fixed it

<div class="container-fixed">
<h1>Account Profile</h1>
<form id="profile-form" class="account-profile" onsubmit="return false">
<div class="form-group">
<label class="form-control-label">Account Name</label>
<input type="text" id="account-name" class="form-control" required />
</div>
<div class="form-group">
<label class="form-control-label">Account Number</label>
<input
type="number"
id="account-number"
class="form-control"
required
readonly
/>
</div>
<label class="form-control-label">
<input
type="checkbox"
id="change-pin"
onclick="displayFormToUpdate()"
Copy link
Copy Markdown
Collaborator

@nwankwo-ikemefuna nwankwo-ikemefuna Feb 22, 2023

Choose a reason for hiding this comment

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

displayFormToUpdate() does not describe what this checkbox does. How about togglePinChangeSectionDisplay(). The idea is that when the checkbox is checked, the PIN change section will show, otherwise it will be hidden.

Also, it's advised to attach an "onchange" event listener to the element instead of directly adding the function to onclick attribute. This is to separate html and javascript properly and give you more control.

Copy link
Copy Markdown
Owner Author

@gabito1451 gabito1451 Feb 27, 2023

Choose a reason for hiding this comment

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

Done

/>
Change Pin</label
>
<div id="pin-update-section">
<div class="form-group">
<label class="form-control-label">Current PIN</label>
<input
type="password"
id="current-pin"
required
Copy link
Copy Markdown
Collaborator

@nwankwo-ikemefuna nwankwo-ikemefuna Feb 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Changing PIN should be optional. You should programmatically add the required attribute if the checkbox is checked, and remove it otherwise. As it is now, you can't submit the form unless the checkbox is checked.

Copy link
Copy Markdown
Owner Author

@gabito1451 gabito1451 Feb 27, 2023

Choose a reason for hiding this comment

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

Done

maxlength="4"
class="form-control"
/>
</div>
<div class="form-group">
<label class="form-control-label">New PIN</label>
<input
type="password"
id="new-pin"
required
Copy link
Copy Markdown
Collaborator

@nwankwo-ikemefuna nwankwo-ikemefuna Feb 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Changing PIN should be optional. You should programmatically add the required attribute if the checkbox is checked, and remove it otherwise. As it is now, you can't submit the form unless the checkbox is checked.

Copy link
Copy Markdown
Owner Author

@gabito1451 gabito1451 Feb 27, 2023

Choose a reason for hiding this comment

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

Done

maxlength="4"
class="form-control"
/>
</div>
<div class="form-group">
<label class="form-control-label">Confirm New PIN</label>
<input
type="password"
id="confirm-new-pin"
required
Copy link
Copy Markdown
Collaborator

@nwankwo-ikemefuna nwankwo-ikemefuna Feb 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Changing PIN should be optional. You should programmatically add the required attribute if the checkbox is checked, and remove it otherwise. As it is now, you can't submit the form unless the checkbox is checked.

Copy link
Copy Markdown
Owner Author

@gabito1451 gabito1451 Feb 27, 2023

Choose a reason for hiding this comment

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

Done

maxlength="4"
class="form-control"
/>
</div>
</div>
<div class="form-group">
<button class="bg-gray-800 text-white p-2 rounded-md" id="update-btn">
Update
</button>
</div>
</form>
<button id="close-btn" class="bg-red-800 text-white p-2 rounded-md">
Close
</button>
</div>
</body>
<script src="./common.js"></script>
<script src="./common-session.js"></script>
<script src="./profile.js"></script>
</html>
62 changes: 62 additions & 0 deletions profile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
const profileForm = document.getElementById("profile-form");
const accountNameElem = document.getElementById("account-name");
const accountNumberElem = document.getElementById("account-number");
const changePinElem = document.getElementById("change-pin");
const currentPinElem = document.getElementById("current-pin");
const newPinElem = document.getElementById("new-pin");
const confirmNewPinElem = document.getElementById("confirm-new-pin");
const closeBtnElem = document.getElementById("close-btn");
const updateBtnElem = document.getElementById("update-btn");
// display of aaccount number
accountNumberElem.value = currentUserAccountNumber;

const allUsers = JSON.parse(localStorage.getItem("MB_USER_ACCOUNTS"));
Copy link
Copy Markdown
Collaborator

@nwankwo-ikemefuna nwankwo-ikemefuna Feb 22, 2023

Choose a reason for hiding this comment

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

Don't we have a function for getting all users defined in common.js? Why do this again?


function displayFormToUpdate() {
const pinUpdateSection = document.getElementById("pin-update-section");

if ((pinUpdateSection.style.display = "none")) {
pinUpdateSection.style.display = "block";
} else {
pinUpdateSection.style.display = "none";
}
}
function updateForm() {
if (
currentPinElem.value !==
getUserByAccountNumber(currentUserAccountNumber).accountPin
) {
alert("Incorrect Pin");
return;
}
if (newPinElem.value !== confirmNewPinElem.value) {
alert("Pins do not match, try again");
return;
}

// finding the current user to update profile

let userToBeUpdated = allUsers.find(
Copy link
Copy Markdown
Collaborator

@nwankwo-ikemefuna nwankwo-ikemefuna Feb 22, 2023

Choose a reason for hiding this comment

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

What is your justification for using let here instead of const?

(user) => user.accountNumber == accountNumberElem.value
);
userToBeUpdated.accountName = accountNameElem.value;
userToBeUpdated.accountPin = newPinElem.value;
// changing the updated properties to a string
const updatedObject = JSON.stringify(allUsers);
// storing it back in local storage
localStorage.setItem("MB_USER_ACCOUNTS", updatedObject);
Copy link
Copy Markdown
Collaborator

@nwankwo-ikemefuna nwankwo-ikemefuna Feb 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

You're not doing it right here. You haven't added the modified user to the users array. You're simply pushing back the same users array to local storage.

See sample of how to update a user's details in deposit.js:
Screenshot at Feb 22 9-00-21 PM

See how it's done on line 31.

Suggested pseudocode:

const currentUserIndex = getUserIndexByAccountNumber(currentUserAccountNumber);
allUsers[currentUserIndex].accountName = the updated account name;
// You need to check that user is updating PIN before you update it, otherwise, it will be updated to empty string
if (user is also changing PIN) {
  allUsers[currentUserIndex].accountPin = the updated account PIN;
}
setLocalStorageArrData("MB_USER_ACCOUNTS", allUsers);

Copy link
Copy Markdown
Owner Author

@gabito1451 gabito1451 Feb 27, 2023

Choose a reason for hiding this comment

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

Done

location.href = "transactions.html";
}
updateBtnElem.addEventListener("click", () => updateForm());

function deleteUserAccount() {
Copy link
Copy Markdown
Collaborator

@nwankwo-ikemefuna nwankwo-ikemefuna Feb 22, 2023

Choose a reason for hiding this comment

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

There should be confirm action incase the user clicks the button by error.

Copy link
Copy Markdown
Collaborator

@nwankwo-ikemefuna nwankwo-ikemefuna Feb 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Isn't closeUserAccount()a more appropriate function name?

Copy link
Copy Markdown
Owner Author

@gabito1451 gabito1451 Feb 27, 2023

Choose a reason for hiding this comment

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

Done

const registeredUsers = localStorage.getItem("MB_USER_ACCOUNTS");
Copy link
Copy Markdown
Collaborator

@nwankwo-ikemefuna nwankwo-ikemefuna Feb 22, 2023

Choose a reason for hiding this comment

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

Don't we have a function for getting all users defined in common.js? Why do this?

Copy link
Copy Markdown
Owner Author

@gabito1451 gabito1451 Feb 27, 2023

Choose a reason for hiding this comment

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

done

console.log(registeredUsers);
let usersArray = JSON.parse(registeredUsers);
Copy link
Copy Markdown
Collaborator

@nwankwo-ikemefuna nwankwo-ikemefuna Feb 22, 2023

Choose a reason for hiding this comment

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

What is your justification for using let here instead of const?

Copy link
Copy Markdown
Owner Author

@gabito1451 gabito1451 Feb 27, 2023

Choose a reason for hiding this comment

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

I've changed it since i am nor re-assigning any variable to a new value

const index = getUserIndexByAccountNumber(currentUserAccountNumber);
usersArray.splice(index, 1);
const newArrayOfUsers = JSON.stringify(usersArray);
Copy link
Copy Markdown
Collaborator

@nwankwo-ikemefuna nwankwo-ikemefuna Feb 22, 2023

Choose a reason for hiding this comment

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

JSON.stringify(usersArray) will produce a string, so calling it newArrayOfUsers is misleading.

Copy link
Copy Markdown
Owner Author

@gabito1451 gabito1451 Feb 27, 2023

Choose a reason for hiding this comment

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

Done

localStorage.setItem("MB_USER_ACCOUNTS", newArrayOfUsers);
// location.href = "/";
}
closeBtnElem.addEventListener("click", () => deleteUserAccount());
3 changes: 3 additions & 0 deletions styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,6 @@ table {
margin-left: 780px;
margin-top: 10px;
}
div#pin-update-section {
display: none;
}
8 changes: 6 additions & 2 deletions transactions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,14 @@ const renderTransactionRow = (transaction) => {
const transactionRowData = document.createElement("td");
transactionRowData.setAttribute("class", "transaction-data");
// if transfer transaction, show beneficiary's name instead of account number
if (rowDataKey === 'beneficiaryAccountNumber' && transaction['beneficiaryAccountNumber']) {
if (
rowDataKey === "beneficiaryAccountNumber" &&
transaction["beneficiaryAccountNumber"]
) {
const beneficiary = getUserByAccountNumber(transaction[rowDataKey]);
const beneficiaryName = beneficiary?.accountName;
transactionRowData.textContent = beneficiaryName || transaction[rowDataKey];
transactionRowData.textContent =
beneficiaryName || transaction[rowDataKey];
} else {
transactionRowData.textContent = transaction[rowDataKey];
}
Expand Down