feat(bank-project): Add Dark Mode, Custom Scrollbar, and Fix Transaction Rendering #1566

hitheswar77 · 2025-10-22T09:59:41Z

Description

Hello! I'm submitting a pull request to improve the user experience and accessibility of the Bank Project solution.

This PR introduces three key changes: two new UI features and one critical bug fix.

✨ Features & Enhancements
Dedicated Dark Mode Toggle:

Adds a theme toggle button to the Dashboard header, allowing users to switch between light and dark modes.

The preference is persisted using localStorage.

Implements the dark mode look using a new :root.dark-mode CSS color scheme, improving user comfort.

Custom Scrollbar Styling (UX):

Implements a custom style for the primary window scrollbar, making it more visually subtle and integrated with the app's color scheme.

🐛 Bug Fix
Fix Transaction History Rendering:

Fixes a bug where transactions were not appearing in the Dashboard table (due to how the DocumentFragment was handled).

The updateElement utility function in app.js has been refactored to correctly append complex DOM nodes (like the transaction rows) while still handling simple text updates (like the balance).

Includes basic date formatting in createTransactionRow for a cleaner display (YYYY-MM-DD to MM/DD/YYYY).

I have ensured all changes adhere to the project's original vanilla JavaScript, HTML, and CSS structure. Please let me know if any adjustments are required!
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

---------------------------------------------------------Hope this contribution is valid--------------------------------------------------

Type of change

[ ✔️] Bug fix (non-breaking change which fixes an issue)
[ ✔️] New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

hitheswar77 · 2025-10-22T10:00:55Z

@microsoft-github-policy-service agree

Copilot

Pull request overview

This PR aims to enhance the Bank Project solution with dark mode support, custom scrollbar styling, and a fix for transaction rendering. However, there are several critical issues that need to be addressed before the changes can be merged.

Key Changes:

Addition of dark mode toggle with localStorage persistence
Refactoring of CSS variables to support light/dark themes
Update to transaction rendering logic in the dashboard
Simplification of HTML structure and removal of accessibility features

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 20 comments.

File	Description
styles.css	Introduces dark mode CSS variables and restructures existing styles, but missing custom scrollbar implementation and contains syntax errors
index.html	Adds theme toggle button with sun/moon icons, but creates duplicate buttons and removes significant accessibility features
app.js	Implements theme toggle logic with localStorage persistence and refactors updateElement function to handle DOM nodes, but has element ID mismatches and missing validation

Critical Issues Found:

Missing semicolons in CSS (lines 220, 235) will cause styling to break
Duplicate theme toggle buttons in the HTML (appears twice on the dashboard)
Missing CSS class - "hidden" class used for icon toggling is not defined
Element ID mismatches - JavaScript references 'description' and 'currency' but HTML uses 'transactions-description' and 'balance-currency'
Custom scrollbar feature is missing despite being mentioned in the PR description
Undefined CSS classes - 'negative' and 'positive' classes are applied but not styled

The transaction rendering fix appears to be on the right track by updating the updateElement function to handle both text and DOM nodes, but the implementation needs refinement.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-19T10:37:19Z

7-bank-project/solution/styles.css

+  left: 0;
+  top: 0;


There are redundant duplicate CSS position properties. Lines 318 and 319 repeat "left: 0;" and "top: 0;" which are already set on lines 315 and 316. These duplicate declarations should be removed.

Suggested change

left: 0;

top: 0;

Copilot · 2025-12-19T10:37:20Z

7-bank-project/solution/index.html

+            <!-- Sun Icon (for Dark mode, hidden by default) -->
+            <svg id="sun-icon" class="w-6 h-6 hidden" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke="currentColor" stroke-width="2">


The CSS class "hidden" is used to toggle icon visibility but this class is never defined in the stylesheet. This will cause the sun/moon icons to both remain visible at the same time, breaking the theme toggle visual feedback.

Copilot · 2025-12-19T10:37:20Z

7-bank-project/solution/app.js

+  // Ensure correct icon is visible on load (needs to wait for DOM elements)
+  // This logic runs again in updateRoute or dashboard refresh to ensure icons are set.


The applyThemeOnLoad function is being called from updateRoute to ensure icons are set correctly, but the DOM elements (sun-icon and moon-icon) may not exist yet when this function runs. The function should check if the elements exist before trying to manipulate them, similar to the check on lines 21-25.

Suggested change

// Ensure correct icon is visible on load (needs to wait for DOM elements)

// This logic runs again in updateRoute or dashboard refresh to ensure icons are set.

// Ensure correct icon is visible on load, if the elements exist

const isDark = html.classList.contains('dark-mode');

const sun = document.getElementById('sun-icon');

const moon = document.getElementById('moon-icon');

if (sun && moon) {

moon.classList.toggle('hidden', isDark);

sun.classList.toggle('hidden', !isDark);

}

Copilot · 2025-12-19T10:37:20Z

7-bank-project/solution/app.js

+  tr.children[0].textContent = displayDate; // Use displayDate
+  tr.children[1].textContent = transaction.object;


The variable name 'object' for the transaction description field is confusing and potentially conflicts with JavaScript's Object keyword. Consider renaming this field to something more descriptive like 'description' or 'label' for better clarity and to avoid potential naming conflicts.

Suggested change

tr.children[0].textContent = displayDate; // Use displayDate

tr.children[1].textContent = transaction.object;

const description = transaction.object;

tr.children[0].textContent = displayDate; // Use displayDate

tr.children[1].textContent = description;

Copilot · 2025-12-19T10:37:20Z

7-bank-project/solution/app.js

+
+  // Optionally highlight positive/negative transactions
+  if (transaction.amount < 0) {
+    tr.classList.add('negative');
+  } else if (transaction.amount > 0) {
+    tr.classList.add('positive');
  }


The classList.add('negative') and classList.add('positive') classes are being added to transaction rows, but these CSS classes are not defined anywhere in the stylesheet. These classes will have no visual effect. Either define the styles or remove this code.

Suggested change

// Optionally highlight positive/negative transactions

if (transaction.amount < 0) {

tr.classList.add('negative');

} else if (transaction.amount > 0) {

tr.classList.add('positive');

}

Copilot · 2025-12-19T10:37:23Z

7-bank-project/solution/app.js


-  // Some markups use a separate currency span; keep it empty when using formatted output
-  setTextByAnyId(['balance-currency', 'currency'], '');
+  updateElement('description', account.description);


The function updateDashboard calls updateElement with 'description' but the HTML template uses 'transactions-description' as the element ID (line 90 of index.html). This mismatch will cause the description not to be displayed. The element ID should be 'transactions-description'.

Suggested change

updateElement('description', account.description);

updateElement('transactions-description', account.description);

Copilot · 2025-12-19T10:37:23Z

7-bank-project/solution/styles.css

-  padding: 0 var(--space-sm);
-  color: var(--muted);
+  top: -0.5em;
+  background-color: var(--white); /* Use var(--white) here as well */


The CSS comment says "Use var(--white) here as well" but the background-color is already using var(--white). This comment is misleading or outdated and should be removed or clarified.

Suggested change

background-color: var(--white); /* Use var(--white) here as well */

background-color: var(--white); /* Match login-content background */

Copilot · 2025-12-19T10:37:24Z

7-bank-project/solution/styles.css

+  transition: background-color 0.3s, color 0.3s; /* Smooth transition */
 }


The transition on the body element includes "background-color 0.3s, color 0.3s" but most other elements in the app don't have transitions. This can cause jarring visual effects where the body transitions smoothly but child elements change abruptly. Consider either adding transitions to other elements or removing this transition for consistency.

Suggested change

transition: background-color 0.3s, color 0.3s; /* Smooth transition */

}

}

}

Copilot · 2025-12-19T10:37:24Z

7-bank-project/solution/styles.css

  color: var(--white);
-  margin: 0 var(--space-sm);
+  vertical-align: middle;
+  margin: 0 var(--space-sm)


Missing semicolon at the end of the margin property. This will cause the CSS rule to be invalid.

Copilot · 2025-12-19T10:37:24Z

7-bank-project/solution/app.js

-  };
+    balance: state.account.balance + data.amount,
+    transactions: [...state.account.transactions, data]
+  }


Avoid automated semicolon insertion (90% of all statements in the enclosing function have an explicit semicolon).

feat(bank-project): Add Dark Mode toggle

1b36207

leestott requested a review from Copilot December 19, 2025 10:31

Copilot started reviewing on behalf of leestott December 19, 2025 10:33 View session

Copilot AI reviewed Dec 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(bank-project): Add Dark Mode, Custom Scrollbar, and Fix Transaction Rendering #1566

feat(bank-project): Add Dark Mode, Custom Scrollbar, and Fix Transaction Rendering #1566

hitheswar77 commented Oct 22, 2025

Uh oh!

hitheswar77 commented Oct 22, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		<!-- Sun Icon (for Dark mode, hidden by default) -->
		<svg id="sun-icon" class="w-6 h-6 hidden" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke="currentColor" stroke-width="2">

		// Ensure correct icon is visible on load (needs to wait for DOM elements)
		// This logic runs again in updateRoute or dashboard refresh to ensure icons are set.

-  // Ensure correct icon is visible on load (needs to wait for DOM elements)
-  // This logic runs again in updateRoute or dashboard refresh to ensure icons are set.
+  // Ensure correct icon is visible on load, if the elements exist
+  const isDark = html.classList.contains('dark-mode');
+  const sun = document.getElementById('sun-icon');
+  const moon = document.getElementById('moon-icon');
+  if (sun && moon) {
+    moon.classList.toggle('hidden', isDark);
+    sun.classList.toggle('hidden', !isDark);
+  }

		tr.children[0].textContent = displayDate; // Use displayDate
		tr.children[1].textContent = transaction.object;

	updateElement('description', account.description);
	updateElement('transactions-description', account.description);

	background-color: var(--white); /* Use var(--white) here as well */
	background-color: var(--white); /* Match login-content background */

		transition: background-color 0.3s, color 0.3s; /* Smooth transition */
		}

feat(bank-project): Add Dark Mode, Custom Scrollbar, and Fix Transaction Rendering #1566

Are you sure you want to change the base?

feat(bank-project): Add Dark Mode, Custom Scrollbar, and Fix Transaction Rendering #1566

Conversation

hitheswar77 commented Oct 22, 2025

Description

Type of change

Uh oh!

hitheswar77 commented Oct 22, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant