Skip to content

JavaScript optimization: eliminate redundancy and improve performance#58

Merged
VatsalSy merged 53 commits intomainfrom
aug-updates
Aug 24, 2025
Merged

JavaScript optimization: eliminate redundancy and improve performance#58
VatsalSy merged 53 commits intomainfrom
aug-updates

Conversation

@VatsalSy
Copy link
Member

Summary

Major JavaScript refactoring to eliminate redundancy and improve maintainability by consolidating duplicate code across multiple files.

Changes Made

🆕 Added New Modules

  • utils.js (8KB) - Centralized platform detection, modal creation, copy functionality, and DOM utilities
  • search-manager.js (7KB) - Unified Fuse.js integration and search database management

🗑️ Removed Redundant Files

  • shortcut-key.js - Completely redundant with platform-utils.js (eliminated)
  • shortcut-key.test.js - Associated test file (eliminated)

🔄 Refactored Core Files

  • main.js - Uses shared Utils.copyToClipboard, removed duplicate implementations
  • command-palette.js - Uses SearchManager for database operations, removed duplicate search code
  • command-data.js - Uses shared modal and search utilities, eliminated duplicate functionality
  • platform-utils.js - Delegates to shared utilities while maintaining backwards compatibility

📄 Updated Layouts

  • default.html, join-us.html, team.html - Updated script loading order, removed deleted references

Performance Improvements

  • ~464 net lines removed (588 deleted, 124 added)
  • 400+ lines of duplicate code eliminated
  • Consolidated 3x duplicate search implementations → 1 cached instance
  • Unified 3x platform detection functions → 1 centralized implementation
  • Merged 2x copy email functions → 1 shared utility

Key Redundancies Eliminated

  1. Platform Detection - 3x duplication → 1 centralized function
  2. Copy Email Functionality - 2x duplication → 1 shared implementation
  3. Search Database Loading - 3x duplication → 1 managed instance
  4. Fuse.js Initialization - 3x duplication → 1 cached instance
  5. Modal Creation - 2x similar patterns → 1 reusable utility

Testing & Validation

  • ✅ All tests passing (44/44 test cases)
  • ✅ Backwards compatibility maintained
  • ✅ No functional changes - pure refactoring
  • ✅ Improved error handling with fallbacks

Technical Details

  • DRY principles implemented throughout
  • Better maintainability - changes only need to be made in one place
  • Enhanced performance - reduced parsing/execution time
  • Proper dependency management - core utilities load before dependent modules

This optimization significantly improves codebase maintainability while preserving all existing functionality.

VatsalSy added 30 commits July 30, 2025 00:42
- Added July 2025 section to both News.md and history.md
- Included link to thesis and GitHub profile
- Maintained 5-item limit in News.md
- Updated PLAN.md to mark task as completed
- Updated .stylelintrc.json to handle modern CSS linting rules
- Excluded vendor CSS files (fontello, academicons) from linting
- Fixed color hex notation, pseudo-element syntax, and other style issues
- Removed conflicting font-size/font shorthand in vendor.css
- Added bsky-embed to allowed custom element types
- All CSS linting tests now pass
- Marked Saumili graduation post as completed
- Marked CSS linting fixes as completed
- Added completed tasks summary section
- Updated timeline to show completed items
- Checked off CI tests in pre-merge checklist
- Fixed markdown formatting for linting compliance
- Replace vatsalsy@comphy-lab.org with vatsal.sanjay@comphy-lab.org
- Add Durham email vatsal.sanjay@durham.ac.uk as secondary contact
- Update email badges and copy buttons in join.html, aboutCoMPhy.md, and teaching pages
- Disabled MD001 rule in markdownlint config to allow h4 Highlights under h3 papers
- Optimized JavaScript in assets/js/main.js
- Add loading="lazy" and decoding="async" to news/history images
- Lazy/async + low fetchpriority for footer logos across layouts
- Add rel="noopener noreferrer" to external links on Join page
- Minor: lazy/decoding for YouTube stats images on About

Improves LCP/CLS and reduces main-thread work without layout changes.
Update footer logos and promo links to use https://www.durham.ac.uk/staff/vatsal-sanjay/ for consistency and accuracy.
…load

Ensure buttons injected from `aboutCoMPhy.md` work by attaching click handlers and aria-labels post-sanitization. Also align quotes to project lint rules.
- Mark lazy-loading and link hardening as done
- Note Durham staff profile link updates
- Set optimization section status to In Progress
- Added .cursor directory to .gitignore to prevent configuration files from being tracked
- Deleted obsolete CSS, general code style, HTML markdown style, image guidelines, JavaScript style, and project structure rules from the .cursor directory for cleanup
Prevent .cursor/**, .claude/**, CLAUDE.md (and .gitignore) from being processed or triggering reloads by adding to _config.yml exclude and .jekyllignore. Reduces noisy auto-regeneration and avoids accidental publish of tooling files.
- Bump nokogiri from 1.18.8 to 1.18.9
- Update all platform-specific versions in Gemfile.lock
- Maintains security and compatibility for HTML parsing
…expand Postdoc, Masters, and add Undergraduate sections for clarity and engagement.

Focus on clearer structure, responsive content, and direct links to project overviews to streamline onboarding.
…fault layout and same permalink. Remove legacy join.html to avoid duplication and improve maintainability.

Fix markdown trailing spaces per markdownlint.
…oin-us/index.md while preserving permalink /join/. Keeps directory naming consistent with the site while ensuring Jekyll outputs the page.
…us/index.md. Page now fully Markdown-compliant.
…d switch _join-us/index.md to use it for proper spacing, typography, and theming.
…le with .email-actions and .email-link for consistent UI. Apply on Join page.
…ainers in aboutCoMPhy.md to match Join page UI.
…graphs, and lists; convert PhD checklist to numbered list and add explicit lead-in. This fixes the stacked left-aligned look.
…sions and update associated tags. Adjust CSS styles for improved typography across headings, paragraphs, and tags for better readability and responsiveness.
…erarchy

- Add dedicated CSS with section cards for PhD, Postdoc, Masters, and Internship positions
- Restructure content into distinct position sections with icons and color coding
- Move research projects to showcase grid at bottom with improved PDF links
- Implement responsive design with mobile-first approach
- Add full dark mode support with appropriate color variables
- Improve typography and spacing for better readability
The heading was displayed twice - once from the layout template
and once from the markdown content. Keep only the template version
for cleaner presentation.
- Group projects into "Soft Matter Singularities" and "Free-Surface Flows" sections
- Add styled category headers with colored accent lines
- Maintain responsive grid layout within each category
- Improve project organization for better research area clarity
…pers and Kramdown attributes; preserve exact styling via existing CSS classes and enable copy-email UI. Improves maintainability without changing appearance.
…ks; invite early contact for planning strong applications.
…ing near-white glyph making icon appear white).
…ing, glyph inherits section accent; dark: white chip, no ring, glyph inherits accent. Remove previous rings/border overrides.
…wesome icon in both themes, inheriting section accent.
…D Scholarships enabling eligible candidates to spend 1 year at CoMPhy Lab.
…isting PhD Positions section; remove duplicate standalone block.
… first-contact guidance and clarify timeline.
- Simplify PhD application guidance with clearer email instructions
- Remove redundant application requirements section
- Add direct links to funding sources (EPSRC DTP, Durham, CSC)
- Include Julia in programming languages list for Bachelor's/Master's projects
- Add soft matter to internship candidate interests
- Improve formatting consistency with periods and blockquotes
- Link numerical techniques to GitHub organization
- Add utils.js with consolidated platform detection, modal creation, and copy functionality
- Add search-manager.js to centralize Fuse.js integration and search database management
- These modules serve as foundation for removing 400+ lines of duplicate code across the codebase
- Implements proper error handling and backwards compatibility
- Delete shortcut-key.js which was completely redundant with platform-utils.js
- Remove associated test file that is no longer needed
- Platform detection functionality is now handled by shared utils.js module
- This eliminates 131 lines of duplicate code
- Update main.js to use Utils.copyToClipboard instead of duplicate implementations
- Refactor command-palette.js to use SearchManager for database operations
- Simplify command-data.js by utilizing shared modal and search utilities
- Remove ~300 lines of duplicate search initialization and modal creation code
- All functionality preserved while eliminating redundancy
- Convert platform-utils.js to use shared Utils module while maintaining backwards compatibility
- Add fallback implementations for cases where Utils module is unavailable
- Preserve existing behavior for any code depending on platform-utils.js
- Reduces redundancy while ensuring smooth migration path
- Add utils.js and search-manager.js to layout script loading order
- Remove references to deleted shortcut-key.js from layouts
- Ensure core utilities load before dependent modules for proper initialization
- Maintain proper script loading sequence for backwards compatibility
- Document completion of JavaScript redundancy elimination project
- Record successful removal of 400+ lines of duplicate code
- Update optimization plan with final results and testing status
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 24, 2025

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Summary by CodeRabbit

  • New Features
    • New “Join Us” section with dedicated layout, project cards, and copy-to-clipboard email actions.
    • Enhanced command palette with centralized search and improved modal UI/accessibility.
  • Performance
    • Site-wide lazy-loading/async decoding for images and deferred scripts; lazy-loaded map on Team page.
  • Improvements
    • Footer Durham links updated to staff profile; refined dark mode and typography across pages.
  • Content
    • Updated News, History, Research entries and teaching contact details.
  • Chores
    • Ignored AI tooling files; adjusted Markdown/CSS lint configs; dependency patch update.

Walkthrough

Removes several .cursor rule documents, updates ignore/lint configs, adds a Join Us collection/layout/CSS, refactors front-end utilities (Utils, SearchManager) and command-palette flows, reorders/deletes legacy scripts/tests, normalizes CSS tokens, updates content/links/images, and bumps a gem dependency.

Changes

Cohort / File(s) Summary
Removed rule docs
.cursor/rules/*
Deletes multiple Cursor rule guideline files: css-style.mdc, general-code-style.mdc, html-markdown-style.mdc, image-guidelines.mdc, javascript-style.mdc, project-structure.mdc.
Ignore & lint config
/.gitignore, /.jekyllignore, /.markdownlint.json, /.stylelintrc.json, /_config.yml
Adds .cursor/.claudeignores and CLAUDE.md exclusion; disables MD001 in markdownlint; updates Stylelint ignoreFiles and rules; addsjoin-uscollection and expands_config.yml` exclude list.
Join Us feature
/_layouts/join-us.html, /_join-us/index.md, /assets/css/join-us.css, /join.html
Adds new layout, collection page, and CSS; deletes legacy join.html (migration to collection-backed page).
Command palette & search refactor
/assets/js/utils.js, /assets/js/search-manager.js, /assets/js/command-data.js, /assets/js/command-palette.js, /assets/js/platform-utils.js, /assets/js/shortcut-key.js, /tests/shortcut-key.test.js, /_layouts/team.html, /_layouts/default.html
Introduces Utils and SearchManager; centralizes modal and search logic; replaces inline palette/search with SearchManager; removes shortcut-key behavior and its tests; updates layouts to load new modules and reorder scripts.
Main JS and UI tweaks
/assets/js/main.js, /assets/js/command-data.js, /assets/js/command-palette.js
Delegate copy/email and platform logic to Utils; wire palette to SearchManager; improve featured papers assembly and copy-button binding; remove legacy globals.
Layouts — footer/link/perf attributes
/_layouts/default.html, /_layouts/history.html, /_layouts/research.html, /_layouts/teaching.html, /_layouts/teaching-course.html
Add loading="lazy" decoding="async" fetchpriority="low" to footer logos; update Durham links to staff profile; add utils.js/search-manager.js before palette.
Team layout and map improvements
/_layouts/team.html
Replace inline command-palette wiring with modular scripts; add theme management, font loading logic, lazy Google Map iframe via IntersectionObserver; remove window.openCommandPalette.
CSS normalization & additions
/assets/css/styles.css, /assets/css/teaching.css, /assets/css/team.css, /assets/css/research.css, /assets/css/shared-news-history.css, /assets/css/vendor.css, /assets/css/command-palette.css, /assets/css/join-us.css
Normalize color tokens (hex shorthand, case), switch pseudo-elements to single-colon, formatting changes, add join-us stylesheet and new email UI classes; mostly non-behavioral.
Content updates
/News.md, /history.md, /_research/index.md, /_teaching/2025-Basilisk101-Madrid.md, /aboutCoMPhy.md
Add July/news content, remove some previous entries, update image lazy-loading attributes, update contact emails and Durham links.
Tests removed
/tests/shortcut-key.test.js
Deletes tests tied to removed shortcut-key.js behavior.
Dependency bump
/Gemfile
Bumps nokogiri from ~> 1.18.8 to ~> 1.18.9.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as User
  participant CP as CommandPalette UI
  participant SM as SearchManager
  participant Fuse as Fuse.js
  participant UI as Utils (Modal / Clipboard)

  U->>CP: open palette / type query
  CP->>SM: searchForCommandPalette(query)
  alt DB not loaded
    SM->>SM: loadSearchDatabase()
    SM->>Fuse: initializeFuse(config)
  end
  SM->>Fuse: search(query)
  Fuse-->>SM: results (scored)
  SM-->>CP: mapped results
  CP->>CP: render results sections
  U->>CP: request help/filter
  CP->>UI: Utils.createModal(content)
  UI-->>U: modal shown (focus + keyboard handled)
Loading
sequenceDiagram
  autonumber
  actor U as User
  participant L as Page Layout
  participant JS as Scripts (utils.js, search-manager.js, command-palette.js)
  participant PU as Platform Utils

  L->>JS: defer-load utils.js, search-manager.js, command-palette.js
  L->>PU: load platform-utils (wrapper)
  JS->>U: update platform-specific UI (⌘/Ctrl)
  U->>JS: click copy email
  JS->>JS: Utils.copyToClipboard(...)
  JS-->>U: feedback (check/tooltip)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • fix-build-script #43 — Overlaps heavily: centralizes utilities/search and integrates SearchManager/Utils with command-palette changes.
  • vs-branch-6 #33 — Related to platform/shortcut-key and command-palette wiring replaced by modular scripts.
  • vs-branch-1 #29 — Related to command-palette implementation and initialization changes.

Poem

I nibble keys and polish code,
New pages bloom along the road.
Search hums tidy, modals bright,
Join Us cards in themed delight.
Logos lazy, emails neat—hop, ship! 🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled
  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3f171b6 and f2d80b6.

📒 Files selected for processing (3)
  • assets/css/vendor.css (1 hunks)
  • assets/js/main.js (12 hunks)
  • assets/js/utils.js (1 hunks)
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch aug-updates

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (19)
_layouts/research.html (1)

277-282: Remove duplicate script includes to avoid double initialization and event binding.

marked, fuse.js, and main.js are included twice (once in the head and again near the end of the body). This risks duplicate listeners, double-rendering, and nondeterministic state in modules that are not idempotent.

Apply this diff to keep a single include for each library (keeping the head versions):

-  <!-- Core scripts -->
-  <script defer src="/assets/js/main.js"></script>
-  <script defer src="https://cdn.jsdelivr.net/npm/marked/marked.min.js"></script>
-  <!-- Search dependencies -->
-  <script defer src="https://cdn.jsdelivr.net/npm/fuse.js@6.6.2"></script>
+  <!-- Core scripts are already loaded in the head; duplicates removed -->

Also applies to: 69-73

assets/css/team.css (1)

261-279: Duplicate @Keyframes “loading-wave” blocks

There are two identical @Keyframes definitions back-to-back. Keep one.

@keyframes loading-wave {
  0% {
    left: -100%;
  }
- 
  100% {
    left: 200%;
  }
}
-
-@keyframes loading-wave {
-  0% {
-    left: -100%;
-  }
-
-  100% {
-    left: 200%;
-  }
-}
assets/js/main.js (2)

391-419: Guard Utils usage and avoid labeling “undefined” email addresses

Same load-order consideration as earlier; also, when data-text is missing, the label becomes “Copy email address undefined”.

-  const images = document.querySelectorAll(
-      '.member-image img[loading="lazy"]'
-    );
+  const images = document.querySelectorAll('.member-image img[loading="lazy"]');
@@
-  const copyButtons = document.querySelectorAll(".copy-btn");
-  copyButtons.forEach((button) => {
-    button.addEventListener("click", function () {
-      Utils.copyToClipboard(this);
-    });
-
-    // Add accessible attributes using utility
-    const emailText =
-      button.getAttribute("data-text") ||
-      button.getAttribute("data-clipboard-text");
-    Utils.initAccessibleButton(button, `Copy email address ${emailText}`);
-  });
+  const copyButtons = document.querySelectorAll(".copy-btn");
+  if (!window.Utils) {
+    console.warn("Utils not available; copy-to-clipboard disabled.");
+  } else {
+    copyButtons.forEach((button) => {
+      button.addEventListener("click", function () {
+        Utils.copyToClipboard(this);
+      });
+      const emailText =
+        button.getAttribute("data-text") ||
+        button.getAttribute("data-clipboard-text") ||
+        "";
+      Utils.initAccessibleButton(button, emailText
+        ? `Copy email address ${emailText}`
+        : "Copy email address");
+    });
+  }

1-426: Ensure utils.js Is Loaded Before main.js in All Layouts

The following layout files either omit utils.js entirely or load main.js without first loading utils.js, which will cause Utils references in main.js to fail at runtime:

_layouts/history.html
• Only loads <script defer src="/assets/js/main.js"> (line 85).
_layouts/research.html
• Loads <script defer src="/assets/js/main.js"> twice (lines 71 and 278) and never loads utils.js.
_layouts/teaching-course.html
• Only loads <script defer src="/assets/js/main.js"> (line 84).
_layouts/teaching.html
• Only loads <script defer src="/assets/js/main.js"> (line 84).

Action items for each file above:

  1. Add <script defer src="/assets/js/utils.js"></script> immediately before every <script defer src="/assets/js/main.js"></script>
  2. (Optional) Remove duplicate main.js includes in research.html.

Example fix snippet:

-  <script defer src="/assets/js/main.js"></script>
+  <script defer src="/assets/js/utils.js"></script>
+  <script defer src="/assets/js/main.js"></script>
assets/css/research.css (3)

16-17: Undefined CSS variable: --color-black-alpha-08

--research-shadow: var(--color-black-alpha-08) references a token that isn’t defined in styles.css (only 05/10/15/20/25/30/40/80 exist). This makes downstream shadows invalid/missing.

-  --research-shadow: var(--color-black-alpha-08);
+  --research-shadow: var(--color-black-alpha-10);

702-705: Misspelled token: --color-primary-alpha-5 should be --color-primary-alpha-05

The missing zero causes var() to resolve to nothing, dropping the declaration.

-.research-content tags span:nth-of-type(3) {
-  background: var(--color-primary-alpha-5);
+.research-content tags span:nth-of-type(3) {
+  background: var(--color-primary-alpha-05);
 }
@@
-.research-content tags span:nth-of-type(n + 4) {
-  background: var(--color-primary-alpha-5);
+.research-content tags span:nth-of-type(n + 4) {
+  background: var(--color-primary-alpha-05);
 }

Also applies to: 710-716


634-641: Unknown type selector ‘tags’ is tripping CI; prefer a class or update Biome config

Per static analysis, tags as a type selector is unknown. If markup uses a custom element, keep it and allowlist in Biome (see .stylelintrc.json comment). Otherwise, switch to a class to be standards-friendly.

Option A (preferred): switch to .tags class selectors:

-.research-content tags {
+.research-content .tags {
@@
-.research-content tags span {
+.research-content .tags span {
@@
-[data-theme="dark"] .research-content tags span {
+[data-theme="dark"] .research-content .tags span {
@@
-[data-theme="dark"] tags span,
-[data-theme="dark"] tags a.tag-link {
+[data-theme="dark"] .tags span,
+[data-theme="dark"] .tags a.tag-link {
@@
-[data-theme="dark"] tags span:hover,
-[data-theme="dark"] tags a.tag-link:hover {
+[data-theme="dark"] .tags span:hover,
+[data-theme="dark"] .tags a.tag-link:hover {
@@
-[data-theme="dark"] tags span.active,
-[data-theme="dark"] tags a.tag-link.active {
+[data-theme="dark"] .tags span.active,
+[data-theme="dark"] .tags a.tag-link.active {
@@
-.research-content tags span:nth-of-type(1) {
+.research-content .tags span:nth-of-type(1) {
@@
-.research-content tags span:nth-of-type(2) {
+.research-content .tags span:nth-of-type(2) {
@@
-.research-content tags span:nth-of-type(3) {
+.research-content .tags span:nth-of-type(3) {
@@
-.research-content tags span:nth-of-type(n + 4) {
+.research-content .tags span:nth-of-type(n + 4) {
@@
-.research-content tags span:hover {
+.research-content .tags span:hover {
@@
-.research-content tags span.active {
+.research-content .tags span.active {
@@
-@media screen and (max-width: 600px) {
-  .research-content tags {
+@media screen and (max-width: 600px) {
+  .research-content .tags {

Option B: keep tags (custom element) and allowlist in Biome as shown in the stylelint review.

Also applies to: 656-668, 670-684, 710-716, 747-751

assets/css/teaching.css (1)

368-378: Replace all :contains() selectors with markup-based classes

The :contains() pseudo-selector is not part of the CSS specification and only works in jQuery. You’ll need to add dedicated classes or attributes in your HTML and target those in your stylesheets.

Please update the following sections:

  • assets/css/teaching.css (lines 368–378)
    Replace the jQuery-only h2:contains("Course Schedule") selectors with a class such as .course-schedule-heading.

  • assets/css/command-palette.css (lines 141–145)
    Replace the nested :has(…:contains("Search Results")) selector with a markup class on the section or its title (e.g. .search-results-section or .search-results-title).

Suggested revisions:

--- a/assets/css/teaching.css
+++ b/assets/css/teaching.css
@@ -365,13 +365,16 @@
 /* Enhanced Course Schedule Container */
- .teaching-content h2:contains("Course Schedule") {
+ /* Add in markup: <h2 class="course-schedule-heading">Course Schedule</h2> */
+.teaching-content h2.course-schedule-heading {
   margin-top: 3rem;
 }

- .teaching-content h2:contains("Course Schedule") + h3,
- .teaching-content h2:contains("Course Schedule") + h3 ~ h3,
- .teaching-content h2:contains("Course Schedule") + h3 ~ h4,
- .teaching-content h2:contains("Course Schedule") + h3 ~ ul {
+ .teaching-content h2.course-schedule-heading + h3,
+ .teaching-content h2.course-schedule-heading + h3 ~ h3,
+ .teaching-content h2.course-schedule-heading + h3 ~ h4,
+ .teaching-content h2.course-schedule-heading + h3 ~ ul {
   position: relative;
   z-index: 1;
 }
--- a/assets/css/command-palette.css
+++ b/assets/css/command-palette.css
@@ -138,10 +138,12 @@
 /* Style for search results section */
- .command-palette-section:has(
-   .command-palette-section-title:contains("Search Results")
- ) {
+ /* Add in markup, e.g.: 
+     <div class="command-palette-section search-results-section">
+       <h3 class="command-palette-section-title">Search Results</h3>
+     </div>
+ */
+ .command-palette-section.search-results-section {
   border-top: 1px solid rgba(104, 35, 109, 0.2);
 }
_teaching/2025-Basilisk101-Madrid.md (1)

160-190: Remove deprecated, page-local clipboard implementation; rely on Utils.copyToClipboard/copyEmail alias.

This in-page implementation uses document.execCommand('copy'), which is deprecated. The PR introduces a centralized clipboard utility. Let’s remove this block after ensuring the layout loads assets/js/utils.js (see layout comment).

Apply:

-<script>
-function copyEmail(button) {
-  const textToCopy = button.getAttribute('data-text');
-  // Create a temporary textarea element to copy from
-  const textarea = document.createElement('textarea');
-  textarea.value = textToCopy;
-  textarea.setAttribute('readonly', '');
-  textarea.style.position = 'absolute';
-  textarea.style.left = '-9999px';
-  document.body.appendChild(textarea);
-  // Select and copy the text
-  textarea.select();
-  document.execCommand('copy');
-  // Remove the temporary element
-  document.body.removeChild(textarea);
-  // Show feedback
-  const originalIcon = button.innerHTML;
-  button.innerHTML = '<i class="fas fa-check"></i>';
-  button.classList.add('copied');
-  // Restore original state after a delay
-  setTimeout(() => {
-    button.innerHTML = originalIcon;
-    button.classList.remove('copied');
-  }, 2000);
-}
-</script>
+<!-- Clipboard behavior provided by global Utils.copyToClipboard / copyEmail alias -->
_layouts/teaching-course.html (3)

131-135: Stray closing </script> tag breaks HTML parsing. Remove it.

There’s an unmatched </script> at Line 134. This will invalidate the document and can abort script execution.

Apply:

   <script defer src="/assets/js/command-palette.js"></script>
 
-
-  </script>

81-89: Duplicate inclusion of command-palette.js. Keep a single deferred include.

The script is loaded at Lines 87 and 131. Double-including can register handlers twice and cause duplicated UI behavior.

Apply either deletion:

-  <script defer src="/assets/js/command-palette.js"></script>

(Keep the earlier one at Line 87.)

Also applies to: 131-132


81-89: Load core utilities (utils.js, search-manager.js) before palette and command-data to match the PR architecture.

This layout does not include assets/js/utils.js or assets/js/search-manager.js, which contradicts the PR’s centralization claims. The page-level copyEmail exists only because these aren’t loaded here.

Apply:

   <!-- JavaScript dependencies -->
+  <!-- Core utilities (must load before palette/data) -->
+  <script defer src="/assets/js/utils.js"></script>
+  <script defer src="/assets/js/search-manager.js"></script>
   <script defer src="https://cdn.jsdelivr.net/npm/marked/marked.min.js"></script>
   <script defer src="https://cdn.jsdelivr.net/npm/fuse.js@6.6.2"></script>
-  <script defer src="/assets/js/main.js"></script>
-  <script defer src="/assets/js/platform-utils.js"></script>
-  <!-- Command palette script before command data -->
-  <script defer src="/assets/js/command-palette.js"></script>
-  <script defer src="/assets/js/command-data.js"></script>
+  <script defer src="/assets/js/main.js"></script>
+  <script defer src="/assets/js/platform-utils.js"></script>
+  <!-- Data before palette to guarantee availability on init -->
+  <script defer src="/assets/js/command-data.js"></script>
+  <script defer src="/assets/js/command-palette.js"></script>
assets/css/command-palette.css (1)

142-148: Invalid CSS: :contains() is not a standard pseudo-class; Biome parse errors confirm. Replace selector.

:.contains() is unsupported in CSS Selectors; Biome reports a parsing error and unknown pseudo-class. This will break the rule and can halt processing in strict pipelines. Use a semantic class or data attribute in markup instead of matching inner text.

Fix (CSS only; requires markup change to add a stable hook on the section that renders search results):

-.command-palette-section:has(
-    .command-palette-section-title:contains("Search Results")
-  ) {
+.command-palette-section.is-search-results {
   border-top: 1px solid rgba(104, 35, 109, 0.2);
   margin-top: 10px;
   padding-top: 10px;
 }

And then, in the HTML/template that renders the “Search Results” section header, add the class:

<div class="command-palette-section is-search-results">
  <div class="command-palette-section-title">Search Results</div></div>
assets/js/command-palette.js (1)

60-75: Avoid stale async search results overriding newer queries.

If the user types quickly, earlier promises can resolve later and overwrite the UI. Track a token to discard stale results.

+// Token to ignore stale async results
+let latestQueryToken = 0;
@@
-function renderCommandResults(query) {
+function renderCommandResults(query) {
+  const myToken = ++latestQueryToken;
@@
-  if (query && query.length >= 3 && window.SearchManager) {
+  if (query && query.length >= 3 && window.SearchManager) {
     // Use the centralized search manager
     window.SearchManager.searchForCommandPalette(query)
       .then((searchResults) => {
+        // Drop stale responses
+        if (myToken !== latestQueryToken) return;
         if (searchResults && searchResults.length > 0) {
           // Add search results to sections
           sections["Search Results"] = searchResults;
 
           // Re-render the UI with search results
           renderSections(sections, resultsContainer);
         }
       })
_layouts/team.html (2)

401-406: Duplicate script includes (main.js, marked, fuse) — remove to prevent double initialization.

main.js is already loaded earlier (Line 81), and marked/fuse are loaded (Lines 79–80). Re-including can duplicate listeners and side effects.

-  <!-- Core scripts -->
-  <script defer src="/assets/js/main.js"></script>
-  <script defer src="https://cdn.jsdelivr.net/npm/marked/marked.min.js"></script>
-  <!-- Search dependencies -->
-  <script defer src="https://cdn.jsdelivr.net/npm/fuse.js@6.6.2"></script>
+  <!-- (removed duplicate script tags; these are already loaded above) -->

444-479: Duplicate DOMContentLoaded image retry block — keep one.

Two near-identical blocks run the same logic, causing redundant retries and observers.

   <script>
-    document.addEventListener('DOMContentLoaded', function() {
-      // Handle image loading
-      const images = document.querySelectorAll('.team-photo');
-      const maxRetries = 3;
-
-      images.forEach(img => {
-        let retries = 0;
-
-        function tryLoadImage() {
-          if (retries < maxRetries && !img.classList.contains('loaded')) {
-            retries++;
-            img.src = img.src + '?' + new Date().getTime(); // Cache bust
-          }
-        }
-
-        // Retry on error
-        img.addEventListener('error', () => {
-          setTimeout(tryLoadImage, 1000 * retries); // Exponential backoff
-        });
-
-        // Force load if image is in viewport
-        if ('IntersectionObserver' in window) {
-          const observer = new IntersectionObserver((entries) => {
-            entries.forEach(entry => {
-              if (entry.isIntersecting && !img.classList.contains('loaded')) {
-                tryLoadImage();
-              }
-            });
-          });
-          observer.observe(img);
-        }
-      });
-
-      // Rest of your existing DOMContentLoaded code...
-    });
+    // (removed duplicate image loading block; single instance retained)
   </script>

Also applies to: 482-518

assets/js/command-data.js (1)

247-253: XSS/HTML injection risk when interpolating tag text into HTML

tag.textContent is safe as a value, but once you interpolate it into an HTML string, any “<”, “>”, “&” in the tag will be interpreted as markup when inserted via innerHTML. While your tags are site-authored, avoiding HTML string interpolation for dynamic text is safer and avoids layout glitches caused by unexpected characters.

Apply the pattern below:

  • Keep the container shell in the HTML string.
  • Append buttons as DOM nodes with textContent set, after the modal is in the DOM.

Diff within nearby lines to introduce the shell container:

-            html += '<div class="tag-filter-container" style="display: flex; flex-wrap: wrap; gap: 10px; margin: 20px 0;">';
+            html += '<div class="tag-filter-container" id="tag-filter-container" style="display: flex; flex-wrap: wrap; gap: 10px; margin: 20px 0;"></div>';

And add the DOM-based population right after you obtain content (see follow-up snippet in the later comment at lines 285-286).

Also applies to: 259-262

_layouts/teaching.html (2)

87-89: Duplicate inclusion of command-palette.js (runs twice, doubles listeners)

command-palette.js is included at Line 87 and again at Line 132. This can register duplicate global key handlers and event listeners, leading to erratic UX and perf cost.

Apply this diff to remove the duplicate:

-  <script defer src="/assets/js/command-palette.js"></script>

Also applies to: 132-132


81-90: Add core utilities before dependent scripts in teaching layout

The teaching.html layout does not inherit from default.html (no YAML front matter specifying layout: default), so it must explicitly include the core utility scripts. Without loading utils.js and search-manager.js first, calls in platform-utils.js, main.js, command-palette.js, and command-data.js will throw at runtime.

Please insert the utility scripts immediately after Fuse.js and before main.js/platform-utils.js:

   <!-- JavaScript dependencies -->
   <script defer src="https://cdn.jsdelivr.net/npm/marked/marked.min.js"></script>
   <script defer src="https://cdn.jsdelivr.net/npm/fuse.js@6.6.2"></script>
+  <script defer src="/assets/js/utils.js"></script>
+  <script defer src="/assets/js/search-manager.js"></script>
   <script defer src="/assets/js/main.js"></script>
   <script defer src="/assets/js/platform-utils.js"></script>
   <!-- Command palette script before command data -->

This ensures window.Utils and window.SearchManager are defined before any dependent code executes.

- Fix CSS syntax issue where content property was declared twice
- Keep only 'content: none' declaration for cleaner reset styles
- Check for Utils availability before using copyToClipboard and initAccessibleButton
- Gracefully disable copy buttons when Utils is unavailable
- Add console warnings for missing dependencies
- Fix quote escaping issues in comments and error messages
- Maintain backwards compatibility with window.copyEmail fallback
Platform Detection:
- Add robust macOS detection with User-Agent Client Hints support
- Exclude touch devices to prevent iPadOS false positives
- Add case-insensitive fallbacks and proper error handling

Modal Accessibility:
- Add ARIA roles, labels, and descriptions
- Implement proper focus management and restoration
- Add keyboard navigation with focus trapping
- Support Escape key to close modal
- Ensure tab cycling within modal boundaries
@VatsalSy VatsalSy merged commit 6002e17 into main Aug 24, 2025
2 of 3 checks passed
@VatsalSy VatsalSy deleted the aug-updates branch August 24, 2025 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant