Remove all alert() in script.js and create a custom and nice user-friendly alert by css/js (#631) #727

michal-duszak · 2025-09-17T01:56:20Z

No description provided.

…endly alert by css/js (brisbanesocialchess#631)

gemini-code-assist

Summary of Changes

Hello @michal-duszak, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly upgrades the application's notification system by replacing the default, blocking browser alerts with a custom-built, aesthetically pleasing, and non-blocking alternative. This change ensures a more consistent and modern user interface for displaying feedback, errors, and important information, thereby enhancing the overall user experience and application responsiveness.

Highlights

Custom Alert System: Introduced a new custom, user-friendly alert system using Nunjucks, JavaScript, and Tailwind CSS, replacing the browser's native alert() function.
Improved User Experience: The new alert system provides visually distinct notifications for success, error, warning, and info messages, with smooth transitions, dismissibility, and configurable durations.
Code Modernization: All instances of native alert() calls in script.js have been replaced with calls to the new custom alert functions (showSuccess, showError, showWarning), standardizing the notification mechanism.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

deepsource-io · 2025-09-17T01:56:37Z

Here's the code health analysis summary for commits 04e766c..4069961. View details on DeepSource ↗.

Analysis Summary

Analyzer	Status	Summary	Link
Python	✅ Success		View Check ↗
Secrets	✅ Success		View Check ↗
Shell	✅ Success		View Check ↗
Docker	✅ Success		View Check ↗
JavaScript	✅ Success	🎯 6 occurences resolved	View Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

gemini-code-assist

Code Review

This pull request does a great job of replacing native alert() calls with a much more user-friendly custom alert system. The implementation is solid, providing different alert types and animations.

I've identified a few areas for improvement. Most importantly, there's a critical security vulnerability (XSS) due to unescaped messages from API responses being rendered as HTML. I've also found a bug where alerts have an extremely long default duration, effectively making them permanent. Additionally, I've included some suggestions to improve code maintainability and consistency, such as refactoring an inline event handler and simplifying a confusing function implementation.

Addressing these points will make the new alert system more secure, robust, and easier to maintain.

frontend/_includes/alert.njk

frontend/assets/scripts/script.js

michal-duszak · 2025-09-17T12:53:36Z

@jbampton I see you've self-assigned this PR. Should I stop working on resolving issues?

jbampton · 2025-09-17T12:55:01Z

I just reassigned you now @michal-duszak thanks

Sorry for the confusion.

michal-duszak · 2025-09-19T22:22:32Z

@jbampton can we move on no matter failing checks?

jbampton · 2025-09-22T13:43:47Z

/gemini review

gemini-code-assist

Code Review

This pull request does a great job of replacing the native alert() calls with a custom, user-friendly alert system. The new alert component is well-structured and provides different styles for success, error, warning, and info messages. The implementation correctly handles message formatting, HTML escaping to prevent XSS, and animations for showing and dismissing alerts.

I've added a few suggestions to improve the new alert system further, focusing on accessibility, robustness, and JavaScript best practices. Specifically, I've recommended using a proper <button> element for the close control to improve accessibility, using crypto.randomUUID() for more reliable unique ID generation, and encapsulating the script in an IIFE to avoid polluting the global namespace.

The changes in script.js to adopt the new alert functions are clean and correct. Overall, this is a solid improvement to the user experience.

frontend/_includes/alert.njk

…tralizing alert functions

…nesocialchess.github.io into feature/631

michal-duszak · 2025-09-22T20:45:57Z

/gemini review

gemini-code-assist

Code Review

This pull request does a great job of replacing the native alert() with a custom and user-friendly alert system. The new AlertSystem is well-structured, handles different message types, and includes nice animations. The code is clean and the implementation is robust, including protection against XSS vulnerabilities. I have a few minor suggestions to improve accessibility, fix a small CSS class issue, and enhance code documentation. Overall, this is a solid improvement.

gemini-code-assist · 2025-09-22T20:47:30Z

frontend/_includes/alert.njk

+            const alertId = 'alert-' + crypto.randomUUID();
+
+            const alertHTML = `
+				<div id="${alertId}" class="w-lg w-full ${config.bg} border ${config.border} ${config.text} px-4 py-3 rounded relative shadow-lg transform transition-all duration-300 ease-in-out translate-x-full opacity-0" role="alert">


The CSS class w-lg is not a standard Tailwind CSS class and will have no effect. It seems you intended to set a maximum width for the alert. I suggest changing it to max-w-lg to constrain the alert's width on larger screens, which improves readability.

<div id="${alertId}" class="max-w-lg w-full ${config.bg} border ${config.border} ${config.text} px-4 py-3 rounded relative shadow-lg transform transition-all duration-300 ease-in-out translate-x-full opacity-0" role="alert">

gemini-code-assist · 2025-09-22T20:47:30Z

frontend/_includes/alert.njk

+				<div id="${alertId}" class="w-lg w-full ${config.bg} border ${config.border} ${config.text} px-4 py-3 rounded relative shadow-lg transform transition-all duration-300 ease-in-out translate-x-full opacity-0" role="alert">
+					<span class="block pr-2">${message}</span>
+					${dismissible ? `
+						<button class="absolute top-2 right-2 cursor-pointer alert-dismiss-button">


For better accessibility and to follow best practices, the dismiss button should have an aria-label attribute to describe its function to screen readers. The <title> element inside an SVG is not consistently supported by all screen reader/browser combinations. Also, adding type="button" prevents it from unintentionally submitting a form if it were ever placed inside one.

<button type="button" aria-label="Close" class="absolute top-2 right-2 cursor-pointer alert-dismiss-button">

gemini-code-assist · 2025-09-22T20:47:30Z

frontend/assets/scripts/script.js

- * Displays an alert message with a list of validation errors.
- * @param {string[]} errors - The list of error messages.
+ * Displays a warning alert message.
+ * @param {string|string[]} message - The list of error messages.


The JSDoc for the message parameter is a bit confusing. Since the function is named showWarning, the documentation should refer to "warning messages" instead of "error messages" to maintain consistency and clarity.

Suggested change

* @param {string|string[]} message - The list of error messages.

* @param {string|string[]} message - The warning message or an array of messages.

BaseMax · 2025-09-28T13:38:33Z

Tests:

BaseMax · 2025-09-28T13:39:10Z

Hey, it was a key feature. @michal-duszak

Many thanks for your help. I will try to pull and test it locally. Also it can be nice if you can share a screenshot of the custom modal.

jbampton · 2025-09-28T13:42:21Z

@BaseMax please don't reference those tests on PRs. See the issue list and please keep up mate.

We are not building the universe in one pr.

@michal-duszak is not responsible for fixing those

michal-duszak · 2025-09-28T16:04:39Z

@BaseMax custom modal screenshots.

Warning:

Error:

EDIT: It's not a modal, it's an alert

Remove all alert() in script.js and create a custom and nice user-fri…

dcbc584

…endly alert by css/js (brisbanesocialchess#631)

michal-duszak requested a review from jbampton as a code owner September 17, 2025 01:56

github-actions bot added the frontend label Sep 17, 2025

gemini-code-assist bot reviewed Sep 17, 2025

View reviewed changes

jbampton self-assigned this Sep 17, 2025

jbampton marked this pull request as draft September 17, 2025 09:34

jbampton added this to Chess Sep 17, 2025

jbampton assigned michal-duszak and unassigned jbampton Sep 17, 2025

jbampton moved this to In Progress in Chess Sep 17, 2025

michal-duszak and others added 2 commits September 19, 2025 05:20

feat(alert): improve security and user experience

fbfd745

Merge branch 'main' into feature/631

99e4756

michal-duszak marked this pull request as ready for review September 22, 2025 13:25

jbampton requested a review from BaseMax September 22, 2025 13:43

gemini-code-assist bot reviewed Sep 22, 2025

View reviewed changes

frontend/_includes/alert.njk Outdated Show resolved Hide resolved

frontend/_includes/alert.njk Outdated Show resolved Hide resolved

frontend/_includes/alert.njk Outdated Show resolved Hide resolved

michal-duszak added 2 commits September 22, 2025 22:44

refactor(alert): enhance alert system by using crypto for IDs and cen…

e53a219

…tralizing alert functions

Merge branch 'feature/631' of https://github.com/michal-duszak/brisba…

9da9f63

…nesocialchess.github.io into feature/631

gemini-code-assist bot reviewed Sep 22, 2025

View reviewed changes

Merge branch 'main' into feature/631

a89b77c

jbampton added this to the September milestone Sep 26, 2025

jbampton linked an issue Sep 26, 2025 that may be closed by this pull request

Remove all alert() in script.js and create a custom and nice user-friendly alert by css/js #631

Open

Merge branch 'main' into feature/631

4069961

jbampton added javascript Pull requests that update javascript code medium-priority nunjucks labels Oct 3, 2025

jbampton modified the milestones: September, Hacktoberfest Oct 3, 2025

jbampton requested a review from prio101 October 4, 2025 21:13

jbampton marked this pull request as draft October 8, 2025 00:00

	* @param {string\|string[]} message - The list of error messages.
	* @param {string\|string[]} message - The warning message or an array of messages.

Uh oh!

Remove all alert() in script.js and create a custom and nice user-friendly alert by css/js (#631) #727

Are you sure you want to change the base?

Remove all alert() in script.js and create a custom and nice user-friendly alert by css/js (#631) #727

Conversation

michal-duszak commented Sep 17, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

deepsource-io bot commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Analysis Summary

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

michal-duszak commented Sep 17, 2025

Uh oh!

jbampton commented Sep 17, 2025

Uh oh!

michal-duszak commented Sep 19, 2025

Uh oh!

jbampton commented Sep 22, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

michal-duszak commented Sep 22, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

BaseMax commented Sep 28, 2025

Uh oh!

BaseMax commented Sep 28, 2025

Uh oh!

jbampton commented Sep 28, 2025

Uh oh!

michal-duszak commented Sep 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

deepsource-io bot commented Sep 17, 2025 •

edited

Loading

michal-duszak commented Sep 28, 2025 •

edited

Loading