Skip to content
Open
Changes from all commits
Commits
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
25 changes: 21 additions & 4 deletions web/virtual_lab/templates/virtual_lab/layout.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
<button class="hover:text-indigo-600 dark:hover:text-indigo-400 transition focus:outline-none focus:ring-2 focus:ring-indigo-500">
{% trans "Physics" %}
</button>
<div class="absolute left-0 mt-2 w-40 bg-white dark:bg-gray-700 border border-gray-200 dark:border-gray-600 rounded-md shadow-lg opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 transition pointer-events-none group-hover:pointer-events-auto group-focus-within:pointer-events-auto">

<div class="absolute left-0 mt-2 w-40 bg-white dark:bg-gray-700 border border-gray-200 dark:border-gray-600 rounded-md shadow-lg opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 transition pointer-events-none group-hover:pointer-events-auto group-focus-within:pointer-events-auto ">
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

z-50 applied only to Chemistry dropdown — Physics dropdown risks being overlaid

The Chemistry dropdown menu (line 38) includes z-50, but the Physics dropdown menu (line 24) does not. Because both containers are position: relative; display: inline-block siblings and the Chemistry container appears later in DOM order, the Chemistry container's default stacking order naturally sits above the Physics dropdown menu. Without an explicit z-index on the Physics dropdown, its items can be obscured by the Chemistry container's stacking context, making those items unreachable.

Add z-50 to the Physics dropdown menu div to match:

🐛 Proposed fix — add `z-50` to Physics dropdown
-            <div class="absolute left-0 mt-2 w-40 bg-white dark:bg-gray-700 border border-gray-200 dark:border-gray-600 rounded-md shadow-lg opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 transition pointer-events-none group-hover:pointer-events-auto group-focus-within:pointer-events-auto ">
+            <div class="absolute left-0 mt-2 w-40 bg-white dark:bg-gray-700 border border-gray-200 dark:border-gray-600 rounded-md shadow-lg opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 transition pointer-events-none group-hover:pointer-events-auto group-focus-within:pointer-events-auto z-50">

Also applies to: 38-38

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/virtual_lab/templates/virtual_lab/layout.html` at line 24, The Physics
dropdown menu div is missing a z-index and can be overlaid by the later
Chemistry dropdown; update the Physics dropdown container (the div with classes
starting "absolute left-0 mt-2 w-40 bg-white...") to include the same z-50 class
used by the Chemistry dropdown so both dropdowns have an explicit stacking
context and cannot obscure each other.

⚠️ Potential issue | 🟡 Minor

Trailing whitespace and extra leading space in the Physics dropdown <div> class

Line 24 has a stray leading space (⎵<div) and a trailing space inside the class string (…pointer-events-auto⎵"). These are cosmetic artifacts from the edit but can cause noise in diffs.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/virtual_lab/templates/virtual_lab/layout.html` at line 24, Remove the
stray leading space before the "<div" and the trailing space inside the class
attribute on the Physics dropdown div (the element with classes ending in
"pointer-events-auto"). Edit the div in virtual_lab/layout.html so it begins
with "<div" (no extra leading whitespace) and the class string has no trailing
space before the closing quote, preserving all other classes and attributes.

<a href="{% url 'virtual_lab:physics_pendulum' %}"
class="block px-4 py-2 text-gray-700 dark:text-gray-300 hover:bg-indigo-50 dark:hover:bg-indigo-900 transition">{% trans "Pendulum" %}</a>
<a href="{% url 'virtual_lab:physics_projectile' %}"
Expand All @@ -29,9 +30,25 @@
class="block px-4 py-2 text-gray-700 dark:text-gray-300 hover:bg-yellow-50 dark:hover:bg-yellow-900 transition">{% trans "Inclined Plane" %}</a>
</div>
</div>
<a href="{% url 'virtual_lab:chemistry_home' %}"
class="hover:text-indigo-600 dark:hover:text-indigo-400 transition">{% trans "Chemistry" %}</a>
<a href="{% url 'virtual_lab:code_editor' %}"
<!-- Chemistry Dropdown -->
<div class="relative group inline-block">
<button class="hover:text-indigo-600 dark:hover:text-indigo-400 transition focus:outline-none focus:ring-2 focus:ring-indigo-500">
{% trans "Chemistry" %}
</button>
<div class="absolute left-0 mt-2 w-40 bg-white dark:bg-gray-700 border border-gray-200 dark:border-gray-600 rounded-md shadow-lg opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 transition pointer-events-none group-hover:pointer-events-auto group-focus-within:pointer-events-auto z-50">
Comment on lines +35 to +38
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing ARIA attributes on dropdown button and menu container

The <button> has no aria-haspopup, no aria-expanded, and no type="button". The dropdown <div> has no role="menu". Screen readers and assistive technologies will not identify this as a popup menu. Per coding guidelines: "Include proper ARIA labels where needed for accessibility."

type="button" also prevents unintended form submission if this template is ever embedded inside a <form>.

♿ Proposed fix — add ARIA attributes
-            <button class="hover:text-indigo-600 dark:hover:text-indigo-400 transition focus:outline-none focus:ring-2 focus:ring-indigo-500">
+            <button type="button"
+                    aria-haspopup="true"
+                    aria-expanded="false"
+                    class="hover:text-indigo-600 dark:hover:text-indigo-400 transition focus:outline-none focus:ring-2 focus:ring-indigo-500">
               {% trans "Chemistry" %}
             </button>
-            <div class="absolute left-0 mt-2 w-40 bg-white dark:bg-gray-700 border border-gray-200 dark:border-gray-600 rounded-md shadow-lg opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 transition pointer-events-none group-hover:pointer-events-auto group-focus-within:pointer-events-auto z-50">
+            <div role="menu"
+                 aria-label="{% trans "Chemistry experiments" %}"
+                 class="absolute left-0 mt-2 w-40 bg-white dark:bg-gray-700 border border-gray-200 dark:border-gray-600 rounded-md shadow-lg opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 transition pointer-events-none group-hover:pointer-events-auto group-focus-within:pointer-events-auto z-50">

Note: aria-expanded toggling between "true"/"false" requires a small JS snippet to be fully effective. Apply the same fix to the Physics dropdown button (line 20) for consistency.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<button class="hover:text-indigo-600 dark:hover:text-indigo-400 transition focus:outline-none focus:ring-2 focus:ring-indigo-500">
{% trans "Chemistry" %}
</button>
<div class="absolute left-0 mt-2 w-40 bg-white dark:bg-gray-700 border border-gray-200 dark:border-gray-600 rounded-md shadow-lg opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 transition pointer-events-none group-hover:pointer-events-auto group-focus-within:pointer-events-auto z-50">
<button type="button"
aria-haspopup="true"
aria-expanded="false"
class="hover:text-indigo-600 dark:hover:text-indigo-400 transition focus:outline-none focus:ring-2 focus:ring-indigo-500">
{% trans "Chemistry" %}
</button>
<div role="menu"
aria-label="{% trans "Chemistry experiments" %}"
class="absolute left-0 mt-2 w-40 bg-white dark:bg-gray-700 border border-gray-200 dark:border-gray-600 rounded-md shadow-lg opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 transition pointer-events-none group-hover:pointer-events-auto group-focus-within:pointer-events-auto z-50">
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/virtual_lab/templates/virtual_lab/layout.html` around lines 35 - 38, Add
proper ARIA attributes and button type to make the dropdown accessible: update
the Chemistry dropdown <button> element to include type="button",
aria-haspopup="true" and aria-expanded="false" (and ensure any JS that
opens/closes the menu toggles aria-expanded between "true" and "false"), and add
role="menu" to the dropdown <div> container; apply the same changes to the
Physics dropdown button element referenced in the template to keep behavior
consistent.

<a href="{% url 'virtual_lab:titration' %}"
class="block px-4 py-2 text-gray-700 dark:text-gray-300 hover:bg-indigo-50 dark:hover:bg-indigo-900 transition">{% trans "Titration" %}</a>
<a href="{% url 'virtual_lab:reaction_rate' %}"
class="block px-4 py-2 text-gray-700 dark:text-gray-300 hover:bg-green-50 dark:hover:bg-green-900 transition">{% trans "Reaction Rate" %}</a>
<a href="{% url 'virtual_lab:solubility' %}"
class="block px-4 py-2 text-gray-700 dark:text-gray-300 hover:bg-yellow-50 dark:hover:bg-yellow-900 transition">{% trans "Solubility" %}</a>
<a href="{% url 'virtual_lab:precipitation' %}"
class="block px-4 py-2 text-gray-700 dark:text-gray-300 hover:bg-purple-50 dark:hover:bg-purple-900 transition">{% trans "Precipitation" %}</a>
<a href="{% url 'virtual_lab:ph_indicator' %}"
class="block px-4 py-2 text-gray-700 dark:text-gray-300 hover:bg-teal-50 dark:hover:bg-teal-900 transition">{% trans "pH Indicator" %}</a>
Comment on lines +39 to +48
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Chemistry dropdown links violate the prescribed Tailwind link color classes

The new links use text-gray-700 dark:text-gray-300 instead of the required text-blue-600 hover:text-blue-800 dark:text-blue-400. As per coding guidelines: "Use Tailwind link classes: text-blue-600 hover:text-blue-800 dark:text-blue-400 for links."

🎨 Proposed fix — apply correct link color classes
-              <a href="{% url 'virtual_lab:titration' %}"
-                 class="block px-4 py-2 text-gray-700 dark:text-gray-300 hover:bg-indigo-50 dark:hover:bg-indigo-900 transition">{% trans "Titration" %}</a>
-              <a href="{% url 'virtual_lab:reaction_rate' %}"
-                 class="block px-4 py-2 text-gray-700 dark:text-gray-300 hover:bg-green-50 dark:hover:bg-green-900 transition">{% trans "Reaction Rate" %}</a>
-              <a href="{% url 'virtual_lab:solubility' %}"
-                 class="block px-4 py-2 text-gray-700 dark:text-gray-300 hover:bg-yellow-50 dark:hover:bg-yellow-900 transition">{% trans "Solubility" %}</a>
-              <a href="{% url 'virtual_lab:precipitation' %}"
-                 class="block px-4 py-2 text-gray-700 dark:text-gray-300 hover:bg-purple-50 dark:hover:bg-purple-900 transition">{% trans "Precipitation" %}</a>
-              <a href="{% url 'virtual_lab:ph_indicator' %}"
-                 class="block px-4 py-2 text-gray-700 dark:text-gray-300 hover:bg-teal-50 dark:hover:bg-teal-900 transition">{% trans "pH Indicator" %}</a>
+              <a href="{% url 'virtual_lab:titration' %}"
+                 class="block px-4 py-2 text-blue-600 hover:text-blue-800 dark:text-blue-400 hover:bg-indigo-50 dark:hover:bg-indigo-900 transition">{% trans "Titration" %}</a>
+              <a href="{% url 'virtual_lab:reaction_rate' %}"
+                 class="block px-4 py-2 text-blue-600 hover:text-blue-800 dark:text-blue-400 hover:bg-green-50 dark:hover:bg-green-900 transition">{% trans "Reaction Rate" %}</a>
+              <a href="{% url 'virtual_lab:solubility' %}"
+                 class="block px-4 py-2 text-blue-600 hover:text-blue-800 dark:text-blue-400 hover:bg-yellow-50 dark:hover:bg-yellow-900 transition">{% trans "Solubility" %}</a>
+              <a href="{% url 'virtual_lab:precipitation' %}"
+                 class="block px-4 py-2 text-blue-600 hover:text-blue-800 dark:text-blue-400 hover:bg-purple-50 dark:hover:bg-purple-900 transition">{% trans "Precipitation" %}</a>
+              <a href="{% url 'virtual_lab:ph_indicator' %}"
+                 class="block px-4 py-2 text-blue-600 hover:text-blue-800 dark:text-blue-400 hover:bg-teal-50 dark:hover:bg-teal-900 transition">{% trans "pH Indicator" %}</a>

As per coding guidelines: "Use Tailwind link classes: text-blue-600 hover:text-blue-800 dark:text-blue-400 for links."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<a href="{% url 'virtual_lab:titration' %}"
class="block px-4 py-2 text-gray-700 dark:text-gray-300 hover:bg-indigo-50 dark:hover:bg-indigo-900 transition">{% trans "Titration" %}</a>
<a href="{% url 'virtual_lab:reaction_rate' %}"
class="block px-4 py-2 text-gray-700 dark:text-gray-300 hover:bg-green-50 dark:hover:bg-green-900 transition">{% trans "Reaction Rate" %}</a>
<a href="{% url 'virtual_lab:solubility' %}"
class="block px-4 py-2 text-gray-700 dark:text-gray-300 hover:bg-yellow-50 dark:hover:bg-yellow-900 transition">{% trans "Solubility" %}</a>
<a href="{% url 'virtual_lab:precipitation' %}"
class="block px-4 py-2 text-gray-700 dark:text-gray-300 hover:bg-purple-50 dark:hover:bg-purple-900 transition">{% trans "Precipitation" %}</a>
<a href="{% url 'virtual_lab:ph_indicator' %}"
class="block px-4 py-2 text-gray-700 dark:text-gray-300 hover:bg-teal-50 dark:hover:bg-teal-900 transition">{% trans "pH Indicator" %}</a>
<a href="{% url 'virtual_lab:titration' %}"
class="block px-4 py-2 text-blue-600 hover:text-blue-800 dark:text-blue-400 hover:bg-indigo-50 dark:hover:bg-indigo-900 transition">{% trans "Titration" %}</a>
<a href="{% url 'virtual_lab:reaction_rate' %}"
class="block px-4 py-2 text-blue-600 hover:text-blue-800 dark:text-blue-400 hover:bg-green-50 dark:hover:bg-green-900 transition">{% trans "Reaction Rate" %}</a>
<a href="{% url 'virtual_lab:solubility' %}"
class="block px-4 py-2 text-blue-600 hover:text-blue-800 dark:text-blue-400 hover:bg-yellow-50 dark:hover:bg-yellow-900 transition">{% trans "Solubility" %}</a>
<a href="{% url 'virtual_lab:precipitation' %}"
class="block px-4 py-2 text-blue-600 hover:text-blue-800 dark:text-blue-400 hover:bg-purple-50 dark:hover:bg-purple-900 transition">{% trans "Precipitation" %}</a>
<a href="{% url 'virtual_lab:ph_indicator' %}"
class="block px-4 py-2 text-blue-600 hover:text-blue-800 dark:text-blue-400 hover:bg-teal-50 dark:hover:bg-teal-900 transition">{% trans "pH Indicator" %}</a>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/virtual_lab/templates/virtual_lab/layout.html` around lines 39 - 48,
Update the Tailwind link color classes on the chemistry dropdown
anchors—specifically the <a> elements pointing to 'virtual_lab:titration',
'virtual_lab:reaction_rate', 'virtual_lab:solubility',
'virtual_lab:precipitation', and 'virtual_lab:ph_indicator'—by replacing the
current "text-gray-700 dark:text-gray-300 hover:bg-... dark:hover:bg-..."
classes with the required link classes "text-blue-600 hover:text-blue-800
dark:text-blue-400" (preserving other layout classes like block, px-4, py-2,
transition as needed).

</div>
</div>
<a href="{% url 'virtual_lab:code_editor' %}"
class="hover:text-indigo-600 dark:hover:text-indigo-400 transition">{% trans "Code Editor" %}</a>
</nav>
</div>
Expand Down