feat(virtual-lab): add dropdown menu for Chemistry experiments#936
Open
Faraz011 wants to merge 1 commit intoalphaonelabs:mainfrom
Open
feat(virtual-lab): add dropdown menu for Chemistry experiments#936Faraz011 wants to merge 1 commit intoalphaonelabs:mainfrom
Faraz011 wants to merge 1 commit intoalphaonelabs:mainfrom
Conversation
Contributor
WalkthroughAdded a Chemistry dropdown menu to the Virtual Lab navigation bar with five submenu items (Titration, Reaction Rate, Solubility, Precipitation, pH Indicator), replacing the previous standalone Chemistry link. Minor structural adjustments made to the Physics dropdown. The Code Editor link position remains unchanged. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/virtual_lab/templates/virtual_lab/layout.html`:
- 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.
- 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.
- Around line 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.
- Around line 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).
| </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 "> |
Contributor
There was a problem hiding this comment.
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.
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.
Comment on lines
+35
to
+38
| <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"> |
Contributor
There was a problem hiding this comment.
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.
Comment on lines
+39
to
+48
| <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> |
Contributor
There was a problem hiding this comment.
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).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR enhances the Virtual Lab navigation by replacing the static "Chemistry" link with a fully functional dropdown menu, providing direct access to all chemistry experiments.
Key Changes:
Replaced the single "Chemistry" navigation link with an interactive dropdown menu.
Added menu items for:
Applied consistent styling (
w-40,z-50, hover states) to match the existing Physics dropdown for a unified user experience.Related issues
Fixes #934
Recording.2026-02-23.053849.mp4
Checklist
Summary by CodeRabbit