Conversation
📝 WalkthroughSummary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis update introduces comprehensive documentation and styling guidelines, adds new layout and history pages, and unifies news/history presentation. It modularizes CSS for about, news, and team sections, refactors JavaScript for content loading and featured research, and updates security attributes on external links. News and research content are restructured for clarity and consistency. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant MainJS
participant Server
User->>Browser: Loads main page
Browser->>MainJS: window.onload
MainJS->>Server: fetch('/aboutCoMPhy.md')
Server-->>MainJS: about markdown
MainJS->>Browser: Render about content
MainJS->>Server: fetch('/News.md')
Server-->>MainJS: news markdown
MainJS->>Browser: Render news content
MainJS->>Browser: Append "Archive" button
MainJS->>Server: fetch('/research/')
Server-->>MainJS: research HTML
MainJS->>Browser: Parse and render featured papers
User->>Browser: Clicks "Copy Email"
Browser->>MainJS: copyEmail(button)
MainJS->>Browser: Copy to clipboard, show feedback
Possibly related PRs
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (26)
assets/js/main.js (1)
58-65: Well-implemented History Archive buttonThe History Archive button implementation is clean and includes proper accessibility attributes (
role="button"andtabindex="0"). This matches the PR objective of adding navigation to the new history page.One suggestion for further enhancement:
- historyBtn.innerHTML = '<i class="fa-solid fa-arrow-right" style="margin-right: 8px; font-size: 1.2em;"></i>History Archive'; + historyBtn.innerHTML = '<i class="fa-solid fa-arrow-right" style="margin-right: 8px; font-size: 1.2em;" aria-hidden="true"></i>History Archive';Adding
aria-hidden="true"to the icon ensures screen readers don't announce it redundantly..windsurf/rules/css-style.md (1)
37-56: Example Media Query Structure is illustrative
The snippet demonstrates mobile-first scaling well. As an optional improvement, you could include an example for the largest desktop breakpoint or show how to override specific component styles.history.md (4)
1-4: Consider adding a permalink or URL frontmatter
Specifying apermalink: /history/can ensure consistent routing and SEO-friendly URLs.
14-23: Flex-row layout is consistent but could lazy-load images
All<img>tags have alt text, which is great. To improve performance, consider addingloading="lazy".
29-37: April news-flex-row structure is sound
Like above, considerloading="lazy"on images for large pages.
38-41: Use semantic<figure>/<figcaption>for image credits
Wrapping the credit in a<figure>and<figcaption>enhances accessibility and semantics:- <div class="news-flex-row">…</div> + <figure class="news-flex-row"> + <img … class="news-image"/> + <figcaption style="…">Image credit: …</figcaption> + </figure>.windsurf/rules/image-guidelines.md (1)
36-45: Specify fenced-code block language for linting
The HTML snippet should be annotated as an HTML code block:- ``` + ```htmlThis addresses markdownlint MD040.
.windsurf/rules/javascript-style.md (1)
20-23: Consider module context for strict mode
ES6 modules are strict by default, so the'use strict'directive may be redundant in module-based code. If your project uses native ES modules or a bundler, you can omit it to reduce boilerplate.🧰 Tools
🪛 LanguageTool
[duplication] ~22-~22: Possible typo: you repeated a word.
Context: ...etwhen reassignment is needed - Never usevar` - Use camelCase for variable and function nam...(ENGLISH_WORD_REPEAT_RULE)
assets/css/styles.css (2)
574-623: Add focus-visible outline for accessibility
The history button currently lacks a visible focus indicator, which hinders keyboard navigation. Add a:focus-visiblerule to meet WCAG standards..s-news__history-btn:focus-visible { outline: 2px solid var(--color-link); outline-offset: 2px; }Also consider replacing hard-coded colors with CSS variables for better theming consistency.
2846-2852: Unify.news-flex-rowspacing values
Thegapis set to20pxhere, but inhistory.cssit’s15px. Align these values or document intentional differences to maintain consistent layouts across news and history sections.CLAUDE.md (1)
30-38: Capitalize Markdown and reconsider strict mode directive
- Change "markdown" to "Markdown" for consistency as a proper noun.
- Keep content files in markdown format where possible + Keep content files in Markdown format where possible
- ES6 modules are strict by default; the
'use strict'directive may not be necessary if your JavaScript runs as modules.🧰 Tools
🪛 LanguageTool
[uncategorized] ~32-~32: You might be missing the article “the” here.
Context: ...n - Use semantic HTML elements - Follow BEM naming convention for CSS classes (e.g....(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[grammar] ~33-~33: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...der__nav-list`) - Keep content files in markdown format where possible ## CSS - Use CSS...(MARKDOWN_NNP)
assets/css/history.css (5)
7-13: Scope typography resets under a container
Overriding globalfont-styleandtext-decorationfor anchors may affect unrelated pages. Prefix selectors with a history page scope (e.g.,.history-news-box a) to isolate these fixes.
15-23: Combine and scope icon style rules
You have two similar selector lists targeting icons. Merge them into a single rule and restrict their scope to the history page to avoid global side effects.
32-38: Use semantic classes over generic selectors
Targeting.news-flex-rowdirectly risks collisions. Adopt a BEM-style or more descriptive class name (e.g.,.s-news-flex__row) to improve clarity and prevent conflicts.
67-78: Swap hard-coded dark theme colors for variables
Hex values in dark theme rules reduce flexibility. Use theme-aware custom properties (e.g.,var(--link-doi-color),var(--news-img-shadow)) consistently for easier maintenance.
80-89: Consider responsive adjustments for.history-news-box
The fixedmax-width: 900pxand uniform padding may overflow on smaller devices. Add a mobile breakpoint or fluid padding to ensure the box scales gracefully._layouts/history.html (10)
10-19: Evaluate critical CSS scope and optimization
Inlining only the header and heading visibility rules is a good start, but you may want to document why these selectors are critical. Consider expanding or extracting additional above-the-fold styles into this block for the fastest first paint.
21-26: Preload critical assets
Preloading your core CSS and JS is excellent. Since this is the history page, you might also preloadhistory.cssif its rules are needed immediately to avoid FOUC.
59-65: Site stylesheet inclusion is correct
All necessary CSS files are included in the right order. If critical styling inhistory.cssimpacts above-the-fold content, you may want to preload it or reorder for performance as noted above.
82-88: Deferred JS dependencies are in order
Deferring the Markdown parser (marked.js), search (fuse.js), and your core scripts reduces render-blocking. Verify that unused libraries aren’t loaded on pages that don’t need them.
90-128: Theme toggle logic is robust
The IIFE correctly handles saved preferences, OS defaults, and toggle events. For maintainability, consider moving this script into its own module (e.g.,theme.js) underassets/js/and loading it withdefer.
164-167: Preloader inclusion
The preloader markup is present. If your main JS also handles preloading visuals, ensure you’re not duplicating functionality or styles in multiple places.
169-174: Remove redundant script block
This inline<script>block is now empty (platform detection and shortcut updates moved toplatform-utils.js). Consider removing it to declutter the markup.
176-229: Enhance navigation context for History
The global nav links are comprehensive but don’t include a “History” menu item or active-state styling. For better UX, add a History link (and highlight it when on this layout) so users know where they are.
231-242: Lazy-loading enhancements
Your approach to injectloading="lazy"on images and wrap iframes is effective. You may want to rename theteaching-contentclass tohistory-contentfor semantic clarity, unless it’s intentionally shared.
293-306: Improve accessibility of command palette modal
Consider adding ARIA attributes to the modal container and input for screen readers, e.g.:<div role="dialog" aria-modal="true" aria-labelledby="command-palette-input"> <input id="command-palette-input" aria-label="Type a command" …>This will make the palette more accessible.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
assets/images/research/S002211202500237X_figAb.pngis excluded by!**/*.png
📒 Files selected for processing (16)
.cursor/rules/rule-writing-guide.mdc(0 hunks).gitignore(1 hunks).windsurf/rules/css-style.md(1 hunks).windsurf/rules/general-code-style.md(1 hunks).windsurf/rules/html-markdown-style.md(1 hunks).windsurf/rules/image-guidelines.md(1 hunks).windsurf/rules/javascript-style.md(1 hunks).windsurf/rules/project-structure.md(1 hunks)CLAUDE.md(1 hunks)News.md(1 hunks)_layouts/history.html(1 hunks)_research/index.md(2 hunks)assets/css/history.css(1 hunks)assets/css/styles.css(2 hunks)assets/js/main.js(1 hunks)history.md(1 hunks)
💤 Files with no reviewable changes (1)
- .cursor/rules/rule-writing-guide.mdc
🧰 Additional context used
🪛 LanguageTool
.windsurf/rules/javascript-style.md
[duplication] ~22-~22: Possible typo: you repeated a word.
Context: ...etwhen reassignment is needed - Never usevar` - Use camelCase for variable and function nam...
(ENGLISH_WORD_REPEAT_RULE)
CLAUDE.md
[uncategorized] ~32-~32: You might be missing the article “the” here.
Context: ...n - Use semantic HTML elements - Follow BEM naming convention for CSS classes (e.g....
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[grammar] ~33-~33: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...der__nav-list`) - Keep content files in markdown format where possible ## CSS - Use CSS...
(MARKDOWN_NNP)
News.md
[uncategorized] ~76-~76: Did you mean “I”?
Context: ...djsent" style="text-decoration: none;"> <i class="fa-brands fa-github" style="font...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~79-~79: Did you mean “I”?
Context: ...ne; color: #0056b3; font-weight: 500;"> <i class="fa-solid fa-file-pdf" style="mar...
(I_LOWERCASE_PREMIUM)
history.md
[uncategorized] ~83-~83: Did you mean “I”?
Context: ...djsent" style="text-decoration: none;"> <i class="fa-brands fa-github" style="font...
(I_LOWERCASE_PREMIUM)
🪛 markdownlint-cli2 (0.17.2)
.windsurf/rules/image-guidelines.md
6-6: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
23-23: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
.windsurf/rules/html-markdown-style.md
6-6: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
🪛 ast-grep (0.31.1)
_layouts/history.html
[warning] 245-247: "This link points to a plaintext HTTP URL. Prefer an encrypted HTTPS URL if possible."
Context:
Note: [CWE-319] Authentication Bypass by Primary Weakness [REFERENCES]
- https://cwe.mitre.org/data/definitions/319.html
(plaintext-http-link-html)
🔇 Additional comments (26)
.gitignore (1)
34-34: LGTM!The line
Thumbs.dbappears to be preserved or just have a formatting change, which aligns with the PR objective of removing unused entries while maintaining existing ignore rules..windsurf/rules/general-code-style.md (1)
1-45: Well-structured code style guidelinesThe general code style guidelines document is clearly organized with appropriate sections covering formatting, organization, and documentation standards. The examples effectively illustrate good and bad practices, which will help maintain consistent code quality across the project.
.windsurf/rules/html-markdown-style.md (1)
1-5: Well-defined HTML/Markdown guidelinesThe HTML and Markdown style guidelines are comprehensive and include excellent examples of semantic HTML usage and BEM naming conventions. This will help maintain consistency across documentation and templates.
Also applies to: 7-44
_research/index.md (2)
50-53: New 2025 publication entry is correctly formatted
The<h3 id="16">block and its accompanying<tags>and badge links follow the established pattern and maintain consistency with other entries.
158-158: Expanded author list update is properly applied
The<h3 id="9">element now includes the additional authors Sen, U., Kant, P., and Lohse, D., matching the corrected citation format..windsurf/rules/css-style.md (3)
12-20: CSS Variables section is clear and concise
Defining colors and typography in:rootusing custom properties aligns with best practices for themeability and maintainability.
22-30: Responsive Design guidelines follow mobile-first best practices
The listed breakpoints cover common device widths. Consider briefly noting that additional breakpoints can be added if future layouts require them.
58-62: Common Pitfalls list is comprehensive
The bullet points cover typical mistakes; no changes needed here..windsurf/rules/project-structure.md (3)
12-20: Repository Structure guidelines are well-defined
Mapping templates to their CSS files and calling out shared assets (search.css/styles.css) provides clarity.
21-25: Documentation Maintenance section is appropriate
Manual control of Jekyll deployment and README upkeep instructions are clear and actionable.
26-29: Common Pitfalls properly highlight gotchas
The listed pitfalls will help prevent documentation drift and mapping errors.history.md (3)
10-13: History entry for May 2025 is accurate and consistent
The markdown list item linking to/research#16and the citation format matches other entries.
27-28: April 2025 award entry is clear
The list item with an inline link follows the established pattern and styling.
42-45: March 2025 list item is formatted correctly
The mix of HTML anchors and markdown is consistent with other entries..windsurf/rules/image-guidelines.md (5)
6-10: Introduction clearly states purpose
The context for image optimization and accessibility is well communicated.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
6-6: Headings must start at the beginning of the line
null(MD023, heading-start-left)
12-19: Image Optimization guidelines are thorough
Covering formats and compression aligns with modern web standards.
20-27: Naming Convention examples are descriptive
The[name]-[descriptor]pattern and examples will help maintain consistency.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
23-23: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
29-33: Accessibility section is comprehensive
Emphasizing meaningfulalttext and emptyalt=""for decoratives is spot on.
48-53: Common Pitfalls list is well-considered
These points will catch typical oversights in image handling..windsurf/rules/javascript-style.md (1)
6-17: Comprehensive and clear JavaScript style guidelines
This document effectively outlines ES6+ feature use, naming conventions, async/await patterns, and event handling best practices.CLAUDE.md (1)
1-8: Comprehensive development commands
The build and development steps are clearly documented. Consider breaking multi-command lines into separate bullets or code blocks for improved readability, but the current setup is concise and functional._layouts/history.html (5)
1-9: Head structure and includes are well-formed
The DOCTYPE declaration,langattribute, and inclusion oftheme-init.htmlestablish a solid foundation for theming and accessibility. No immediate issues detected.
27-37: Favicon links are correctly configured
You’ve covered all major platforms and included theme-color meta tags. This implementation looks complete and accurate.
44-57: Font loading strategy looks solid
Usingfont-display: swap, the print/onload media hack, and a<noscript>fallback ensures fonts load efficiently and degrade gracefully.
133-160: Conditional Font Awesome loading is effective
Loading local CSS during development and the Kit in production—with error handling and version constants—provides flexibility and resilience. No changes needed here.
307-315: Global keyboard shortcut registration is appropriate
Thekeydownlistener for Ctrl/Cmd+K is implemented correctly. Ensure thatwindow.openCommandPalette()is defined and error-handled in your deferredcommand-palette.js.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
_layouts/history.html (3)
38-43: Meta description/keywords still describe “Courses” not “History”
Same issue flagged in the previous review – please update these tags for SEO accuracy.
67-81: Structured-data schema mismatches page intentThe embedded JSON-LD declares
@type:"Course"and points to/teaching; switch toCollectionPage(orWebPage) and/historyto avoid SEO penalties.
245-248:⚠️ Potential issueUse HTTPS for external links to avoid mixed-content warnings
http://basilisk.fr/sandbox/vatsal/should be served over HTTPS.-<a href="http://basilisk.fr/sandbox/vatsal/" target="_blank"> +<a href="https://basilisk.fr/sandbox/vatsal/" target="_blank" rel="noopener noreferrer">🧰 Tools
🪛 ast-grep (0.31.1)
[warning] 246-248: "This link points to a plaintext HTTP URL. Prefer an encrypted HTTPS URL if possible."
Context:
Note: [CWE-319] Authentication Bypass by Primary Weakness [REFERENCES]
- https://cwe.mitre.org/data/definitions/319.html(plaintext-http-link-html)
News.md (1)
14-15: Inline styles and<strong>tags should be replaced with classes/Markdown boldThe same clean-up proposed earlier still applies: move
style="margin-right … font-size …"into CSS (e.g.,.icon-inline) and use**bold**instead of<strong>.Also applies to: 40-42, 70-73, 76-81
🧹 Nitpick comments (3)
assets/js/main.js (1)
5-7:"use strict"is redundant inside an IIFEModern browsers automatically apply strict-mode to script modules; for plain scripts an IIFE already constrains scope and the directive adds no value.
Feel free to drop it to cut three bytes and appease Biome’snoRedundantUseStrictrule.🧰 Tools
🪛 Biome (1.9.4)
[error] 6-6: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
history.md (1)
12-23: Replace<strong>and inline icon styles with semantic Markdown and CSSFollowing the new style guides, prefer Markdown bold (
**Author**) and centralise icon spacing/sizing in the shared stylesheet (.icon-inline,.news-icon) instead of per-elementstyleattributes.assets/css/team.css (1)
29-34: Minor duplication of horizontal spacing declarations
margin: 2rem 0;followed by explicitmargin-left/right:auto;works but is slightly brittle.
Consider one declaration (margin: 2rem auto;) for brevity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
News.md(1 hunks)_layouts/default.html(1 hunks)_layouts/history.html(1 hunks)_layouts/team.html(2 hunks)assets/css/about-layout.css(1 hunks)assets/css/history.css(1 hunks)assets/css/shared-news-history.css(1 hunks)assets/css/styles.css(5 hunks)assets/css/team.css(4 hunks)assets/js/main.js(1 hunks)history.md(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- _layouts/default.html
- _layouts/team.html
- assets/css/about-layout.css
- assets/css/shared-news-history.css
🚧 Files skipped from review as they are similar to previous changes (2)
- assets/css/history.css
- assets/css/styles.css
🧰 Additional context used
🪛 Biome (1.9.4)
assets/js/main.js
[error] 6-6: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
[error] 115-115: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 251-251: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 LanguageTool
News.md
[uncategorized] ~76-~76: Did you mean “I”?
Context: ...> <i class="fa-brands fa-github" style="font...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~79-~79: Did you mean “I”?
Context: ...ay: inline-flex; align-items: center;"> <i class="fa-solid fa-file-pdf" style="mar...
(I_LOWERCASE_PREMIUM)
history.md
[uncategorized] ~83-~83: Did you mean “I”?
Context: ...> <i class="fa-brands fa-github" style="font...
(I_LOWERCASE_PREMIUM)
🪛 ast-grep (0.31.1)
_layouts/history.html
[warning] 246-248: "This link points to a plaintext HTTP URL. Prefer an encrypted HTTPS URL if possible."
Context:
Note: [CWE-319] Authentication Bypass by Primary Weakness [REFERENCES]
- https://cwe.mitre.org/data/definitions/319.html
(plaintext-http-link-html)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
_layouts/history.html (3)
38-43: Update SEO metadata to reflect history archive.The current
descriptionandkeywordstarget course content, but this layout renders a history/timeline page. Update these to accurately describe the history archive.- <meta name="description" content="Courses and educational resources offered by the Computational Multiphase Physics Laboratory"> - <meta name="keywords" content="computational physics courses, physics education, multiphase physics training, scientific courses"> + <meta name="description" content="Laboratory milestones, publications, and news archive of the Computational Multiphase Physics Laboratory"> + <meta name="keywords" content="physics research history, laboratory milestones, research publications, multiphase physics archive, comphy lab news">
67-82: Structured data schema mismatch.The JSON-LD defines a
Coursewithurl: "{{ site.url }}/teaching", which does not match the history archive context.{ "@context": "https://schema.org", - "@type": "Course", - "name": "Computational Multiphase Physics Courses", - "description": "Educational resources and courses related to computational multiphase physics.", + "@type": "CollectionPage", + "name": "Computational Multiphase Physics History Archive", + "description": "Timeline of laboratory milestones, publications, and news from the Computational Multiphase Physics Laboratory.", "provider": { "@type": "Organization", "name": "Computational Multiphase Physics Laboratory", "sameAs": "{{ site.url }}" }, - "url": "{{ site.url }}/teaching" + "url": "{{ site.url }}/history" }
247-248:⚠️ Potential issueUpdate to secure HTTPS URL for Basilisk link.
The current link uses an insecure HTTP protocol, which could lead to mixed content warnings or security issues.
- <a href="http://basilisk.fr/sandbox/vatsal/" target="_blank" rel="noopener noreferrer"> + <a href="https://basilisk.fr/sandbox/vatsal/" target="_blank" rel="noopener noreferrer">🧰 Tools
🪛 ast-grep (0.31.1)
[warning] 246-248: "This link points to a plaintext HTTP URL. Prefer an encrypted HTTPS URL if possible."
Context:
Note: [CWE-319] Authentication Bypass by Primary Weakness [REFERENCES]
- https://cwe.mitre.org/data/definitions/319.html(plaintext-http-link-html)
🧹 Nitpick comments (10)
index.html (2)
65-98: Consider moving aggressive style overrides to a CSS file.The style block uses multiple
!importantrules to forcibly override existing styles, which can make debugging and maintenance difficult. While this approach works for isolation, consider moving these overrides to a dedicated CSS file with appropriate specificity to maintain better separation of concerns.- <!-- Adding a style block with !important rules to override any section styling --> - <style> - /* Global override to remove ANY lines and borders from featured wrapper and children */ - #featured-wrapper, - #featured-wrapper *, - #featured-wrapper::before, - #featured-wrapper::after, - #featured-wrapper *::before, - #featured-wrapper *::after { - border: none !important; - border-top: none !important; - border-bottom: none !important; - box-shadow: none !important; - } - - /* Completely hide any ::after pseudo-elements that might be adding lines */ - #featured-wrapper::after, - #featured-wrapper *::after { - display: none !important; - content: none !important; - height: 0 !important; - background: transparent !important; - opacity: 0 !important; - visibility: hidden !important; - border: none !important; - } - - /* Extra strong reset for the wrapper */ - #featured-wrapper { - position: relative !important; - z-index: 1 !important; - overflow: visible !important; - } - </style>Instead, add these styles to a dedicated CSS file like
assets/css/featured-research.cssand link it in the head section.
100-119: Move inline styles to external CSS file for better maintainability.The new featured wrapper uses extensive inline styling which makes the HTML harder to read and maintain. Consider moving these styles to an external CSS file and applying classes instead.
- <div id="featured-wrapper" style="padding: 2rem 2rem; background: var(--color-background); display: flex; align-items: flex-start; position: relative; min-height: 50vh; z-index: 1; border: none !important; overflow: visible;"> - <div style="width: 100%; max-width: 1400px; margin: 0 auto; border: none !important; position: relative; z-index: 2;"> - <div style="text-align: center; margin-bottom: 4rem; border: none !important;"> - <h2 style="font-size: 3.6rem; color: var(--color-heading-h1); margin-bottom: 0.5rem; text-transform: uppercase; letter-spacing: 0.1em; font-weight: 500; border: none !important;"> - Featured Research - </h2> + <div id="featured-wrapper" class="featured-section"> + <div class="featured-container"> + <div class="featured-header"> + <h2 class="featured-title"> + Featured Research + </h2>This can be paired with appropriate CSS in your
shared-news-history.cssor a newfeatured-research.cssfile.assets/js/main.js (4)
5-6: Remove redundant "use strict" directive.Modern JavaScript modules automatically run in strict mode, making this directive unnecessary.
(function (html) { - "use strict";🧰 Tools
🪛 Biome (1.9.4)
[error] 6-6: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
117-118: Use optional chaining for cleaner code.Instead of using the Boolean AND operator to check if
nextElexists before accessing its properties, use optional chaining for more concise code.- return nextEl && nextEl.textContent.includes("Featured"); + return nextEl?.textContent.includes("Featured");🧰 Tools
🪛 Biome (1.9.4)
[error] 117-117: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
253-258: Use optional chaining for cleaner code.Replace the Boolean AND pattern with optional chaining for more concise and readable code.
- if (nav && nav.classList.contains("is-active")) { + if (nav?.classList.contains("is-active")) {🧰 Tools
🪛 Biome (1.9.4)
[error] 253-253: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
370-416: Refactor clipboard functionality to reduce code duplication.The current implementation has duplicated code for success handling between the promise and the fallback path. Consider extracting the common success handling logic into a separate function.
window.copyEmail = function (button) { const text = button.getAttribute("data-text") || button.getAttribute("data-clipboard-text"); + + function handleCopySuccess(btn) { + const icon = btn.querySelector("i"); + btn.classList.add("copied"); + icon.classList.remove("fa-copy"); + icon.classList.add("fa-check"); + + setTimeout(() => { + btn.classList.remove("copied"); + icon.classList.remove("fa-check"); + icon.classList.add("fa-copy"); + }, 2000); + } navigator.clipboard .writeText(text) .then(() => { - const icon = button.querySelector("i"); - button.classList.add("copied"); - icon.classList.remove("fa-copy"); - icon.classList.add("fa-check"); - - setTimeout(() => { - button.classList.remove("copied"); - icon.classList.remove("fa-check"); - icon.classList.add("fa-copy"); - }, 2000); + handleCopySuccess(button); }) .catch((err) => { console.error("Copy failed:", err); // Fallback for older browsers const textarea = document.createElement("textarea"); textarea.value = text; textarea.style.position = "fixed"; textarea.style.opacity = "0"; document.body.appendChild(textarea); textarea.select(); try { document.execCommand("copy"); - button.classList.add("copied"); - const icon = button.querySelector("i"); - icon.classList.remove("fa-copy"); - icon.classList.add("fa-check"); - - setTimeout(() => { - button.classList.remove("copied"); - icon.classList.remove("fa-check"); - icon.classList.add("fa-copy"); - }, 2000); + handleCopySuccess(button); } catch (err) { console.error("Fallback failed:", err); } document.body.removeChild(textarea); }); };assets/css/team.css (2)
87-105: Use theme variable for underline color
The pseudo-element on.s-team__desc h1:afterhardcodesvar(--color-purple-dark)instead of the theme-awarevar(--team-heading-color)..s-team__desc h1:after { - background: var(--color-purple-dark); + background: var(--team-heading-color); } [data-theme="dark"] .s-team__desc h1:after { - background: var(--color-purple-light); + background: var(--team-heading-color); }
113-123: Consider auto-fit grid for flexibility
Currently.team-gridusesrepeat(4, 1fr)and explicit breakpoints. You might adoptrepeat(auto-fit, minmax(250px, 1fr))for more fluid resizing, reducing the number of media queries.assets/css/teaching.css (2)
5-12: Unused teaching-heading-color variable
The custom property--teaching-heading-coloris defined but never used—headings now reference--color-heading-h1. Consider removing or repurposing it to avoid dead code.
16-24: Unused dark theme variable
Similarly,--teaching-heading-colorin the dark theme block isn’t applied anywhere. Clean up to prevent confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
_layouts/default.html(4 hunks)_layouts/history.html(1 hunks)_layouts/research.html(2 hunks)_layouts/teaching-course.html(2 hunks)_layouts/teaching.html(2 hunks)_layouts/team.html(20 hunks)_teaching/2025-Basilisk101-Madrid.md(1 hunks)_team/index.md(7 hunks)aboutCoMPhy.md(1 hunks)assets/css/research.css(7 hunks)assets/css/shared-news-history.css(1 hunks)assets/css/teaching.css(5 hunks)assets/css/team.css(3 hunks)assets/js/main.js(1 hunks)index.html(3 hunks)join.html(1 hunks)
✅ Files skipped from review due to trivial changes (7)
- aboutCoMPhy.md
- join.html
- _teaching/2025-Basilisk101-Madrid.md
- _layouts/teaching.html
- _layouts/research.html
- _layouts/teaching-course.html
- assets/css/shared-news-history.css
🚧 Files skipped from review as they are similar to previous changes (2)
- _layouts/default.html
- _layouts/team.html
🧰 Additional context used
🪛 Biome (1.9.4)
assets/css/team.css
[error] 83-83: Unexpected shorthand property margin after margin-bottom
(lint/suspicious/noShorthandPropertyOverrides)
[error] 390-390: Unexpected shorthand property margin after margin-top
(lint/suspicious/noShorthandPropertyOverrides)
assets/js/main.js
[error] 6-6: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
[error] 117-117: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 253-253: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 ast-grep (0.31.1)
_layouts/history.html
[warning] 246-248: "This link points to a plaintext HTTP URL. Prefer an encrypted HTTPS URL if possible."
Context:
Note: [CWE-319] Authentication Bypass by Primary Weakness [REFERENCES]
- https://cwe.mitre.org/data/definitions/319.html
(plaintext-http-link-html)
🪛 LanguageTool
_team/index.md
[inconsistency] ~24-~24: The abbreviation is usually written “Ph.D.”.
Context: ..._CV.pdf){: .cv-button} ## Ayush Dixit (Ph.D) [<i class="fa-brands fa-github" style=...
(PH_D)
[inconsistency] ~37-~37: The abbreviation is usually written “Ph.D.”.
Context: ..., Respiratory Drops. ## Aman Bhargava (Ph.D) [<i class="fa-brands fa-github" style=...
(PH_D)
🔇 Additional comments (28)
assets/js/main.js (1)
71-73: Good null check implementation.The null check for
newsContentbefore appending the history button is good defensive programming that prevents potential TypeError exceptions.assets/css/team.css (6)
1-46: Well-structured CSS variable definitions
The base theme custom properties cover a comprehensive palette, alpha variants, and spacing, following the style guidelines. Variables are named consistently and will support maintainability and theming.
48-62: Well-defined dark theme overrides
Dark-mode custom properties correctly override only the necessary variables, preserving the base theme defaults. This aligns with the project’s theming strategy.
64-76: Consistent team page layout
The.s-teamcontainer uses a mobile-first, flex-based vertical layout with full-viewport support. Padding, centering, and theming are handled cleanly.
306-314: Browser support for:has()selector
The rules centering “Joint with” text use the CSS relational pseudo-class:has(), which isn’t fully supported in all browsers. Consider adding a utility class or a small JS polyfill to ensure consistency.Would you like assistance adding a class-based fallback?
459-468: Well-implemented map section styles
The new.s-mapsection follows the same theme variables and layout conventions, with proper centering and responsive constraints. Looks solid.
608-769: Comprehensive responsive breakpoints
Media queries at 1700px, 1300px, 900px, 768px, and 500px adapt the layout and typography well, adhering to the mobile-first approach._team/index.md (9)
8-13: Removed trailing whitespace from heading
The heading## Vatsal Sanjay (PI)no longer has extraneous spaces. Excellent cleanup for consistency.🧰 Tools
🪛 LanguageTool
[uncategorized] ~9-~9: Did you mean “I”?
Context: ...ck;">](https://github.com/VatsalSy) [<i class="ai ai-google-scholar-square" sty...(I_LOWERCASE_PREMIUM)
24-29: Removed trailing whitespace from heading
The heading## Ayush Dixit (Ph.D)has been trimmed. Thank you for keeping headings clean.🧰 Tools
🪛 LanguageTool
[inconsistency] ~24-~24: The abbreviation is usually written “Ph.D.”.
Context: ..._CV.pdf){: .cv-button} ## Ayush Dixit (Ph.D) [<i class="fa-brands fa-github" style=...(PH_D)
37-42: Removed trailing whitespace from heading
The heading## Aman Bhargava (Ph.D)has been normalized.🧰 Tools
🪛 LanguageTool
[inconsistency] ~37-~37: The abbreviation is usually written “Ph.D.”.
Context: ..., Respiratory Drops. ## Aman Bhargava (Ph.D) [<i class="fa-brands fa-github" style=...(PH_D)
51-55: Removed trailing whitespace from heading
The heading## Jnandeep Talukdar (M.Sc.)is now tidy.🧰 Tools
🪛 LanguageTool
[uncategorized] ~52-~52: Did you mean “I”?
Context: ...k;">](https://github.com/spilltheT) [<i class="ai ai-google-scholar-square" sty...(I_LOWERCASE_PREMIUM)
60-63: Updated Ph.D. start date
Changed the Ph.D. start month to Jun 2025 for Jnandeep Talukdar. The timeline is now accurate.
66-70: Removed trailing whitespace from heading
The heading## Saumili Jana (M.Sc.)is now cleanly formatted.
73-76: Updated Ph.D. start date
Changed the Ph.D. start month to Jun 2025 for Saumili Jana.
184-188: Removed trailing whitespace from heading
The heading## Radu Cimpeanuhas no trailing spaces.🧰 Tools
🪛 LanguageTool
[uncategorized] ~185-~185: Did you mean “I”?
Context: ...J&hl=en&inst=5726176096060060532&oi=ao) [<i class="fa-brands fa-x-twitter" style="f...(I_LOWERCASE_PREMIUM)
239-242: Removed trailing whitespace from heading
The heading## Stéphane Zaleskiis properly trimmed.assets/css/research.css (8)
5-5: Use of CSS variable fallback for H1 headings
Replacing the fixed heading color with--color-heading-h1and a fallback enhances maintainability and theming.
18-18: Dark theme H1 heading color uses fallback
Good consistency: dark theme heading now uses--color-heading-h1with a fallback.
49-50: Text shadow uses CSS variable
The dark-mode bold text shadow now references--color-white-alpha-30, adhering to the variable-based styling guidelines.
53-56: Inset box-shadow uses variable fallback
Switching to--color-black-alpha-30for the inner shadow is on-point and consistent.
155-158: Year section heading color standardized
Changed H2 headings to use--color-heading-h2, aligning with other sections.
173-178: Paper title heading color standardized
Updated H3 headings to use--color-heading-h3, promoting style consistency.
371-375: Print style heading color
The print stylesheet now uses--color-near-blackfor H1, ensuring legibility when printing.
513-515: Verify definition of--color-secondary
The TOC heading uses--color-secondary, but it’s not defined in this file. Please confirm that this variable is declared globally.assets/css/teaching.css (4)
40-45: Solid color for main heading
Replacing the gradient with--color-heading-h1simplifies styling and improves accessibility.
53-57: Teaching content H1 styling
Increasing font-size and using--color-heading-h1aligns with the new design direction.
59-64: Teaching content H2 styling
Refactoring to--color-heading-h2with a sensible fallback ensures consistency across sections.
81-85: Teaching content H3 styling
Switching to--color-heading-h3with fallback is in line with the research and news sections.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (10)
assets/css/team.css (1)
521-536: Reduce verbosity in override selectors
You’ve added many nearly identical selectors with!importantto remove autogenerated:afterelements. Consider scoping the rule to the map container once and using:is()or a single selector list to simplify maintenance.For example:
.s-map :is(h1, h2)::after { content: none !important; display: none !important; /* ... */ }_team/index.md (4)
24-24: Append missing period in “Ph.D.”
The heading is written## Ayush Dixit (Ph.D)but should usePh.D.for consistency.-## Ayush Dixit (Ph.D) +## Ayush Dixit (Ph.D.)🧰 Tools
🪛 LanguageTool
[inconsistency] ~24-~24: The abbreviation is usually written “Ph.D.”.
Context: ..._CV.pdf){: .cv-button} ## Ayush Dixit (Ph.D) [<i class="fa-brands fa-github" style=...(PH_D)
37-37: Append missing period in “Ph.D.”
Similarly, update## Aman Bhargava (Ph.D)to include the trailing period.-## Aman Bhargava (Ph.D) +## Aman Bhargava (Ph.D.)🧰 Tools
🪛 LanguageTool
[inconsistency] ~37-~37: The abbreviation is usually written “Ph.D.”.
Context: ..., Respiratory Drops. ## Aman Bhargava (Ph.D) [<i class="fa-brands fa-github" style=...(PH_D)
60-60: Normalize “starting Jun 2025” phrasing
Use full month names and capitalize consistently. Consider:- - Ph.D. Student, Phys. Fluids - Univ. Twente / **starting** Jun 2025 + - Ph.D. Student, Phys. Fluids - Univ. Twente / **Starting June 2025**
73-73: Normalize “starting Jun 2025” phrasing
Same adjustment needed for Saumili Jana’s entry.- - Ph.D. Student, Phys. Fluids - Univ. Twente / **starting** Jun 2025 + - Ph.D. Student, Phys. Fluids - Univ. Twente / **Starting June 2025**assets/js/main.js (5)
5-7: Remove redundant "use strict" directive.JavaScript modules are automatically in strict mode without requiring the directive.
-(function (html) { - "use strict"; +(function (html) {🧰 Tools
🪛 Biome (1.9.4)
[error] 6-6: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
110-118: Use optional chaining for safer property access.Using optional chaining would make this code more robust against unexpected DOM structures.
const featuredSections = Array.from(paperSections).filter((section) => { // Find the next tags element let nextEl = section.nextElementSibling; while (nextEl && !nextEl.matches("tags")) { nextEl = nextEl.nextElementSibling; } - return nextEl && nextEl.textContent.includes("Featured"); + return nextEl?.textContent.includes("Featured"); });🧰 Tools
🪛 Biome (1.9.4)
[error] 117-117: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
192-199: Enhance paper navigation with fall-back URL parameters.When the paper number extraction fails, you redirect to the general research page. Consider adding a query parameter with the paper title to help users locate the specific paper.
if (paperNumber) { // Navigate to research page with the paper ID window.location.href = `/research/#${paperNumber}`; } else { - window.location.href = "/research/"; + // Fallback: Use paper title as search parameter + const cleanTitle = title.textContent.trim(); + window.location.href = `/research/?search=${encodeURIComponent(cleanTitle)}`; }
250-256: Use optional chaining for safer property access.Similar to the previous suggestion, use optional chaining for better code robustness.
document.addEventListener("click", function (e) { - if (nav && nav.classList.contains("is-active")) { + if (nav?.classList.contains("is-active")) { // Check if click is outside nav and not on menu toggle if (!nav.contains(e.target) && !menuToggle.contains(e.target)) { nav.classList.remove("is-active"); } } });🧰 Tools
🪛 Biome (1.9.4)
[error] 251-251: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
368-414: Consider consolidating duplicate email copy functionality.There's significant code duplication between this global function and the event handlers at lines 323-366. Consider refactoring to use this global function from the event handlers as well.
// Email copy functionality const copyButtons = document.querySelectorAll(".copy-btn"); copyButtons.forEach((button) => { - button.addEventListener("click", function () { - const textToCopy = this.getAttribute("data-clipboard-text"); - const textarea = document.createElement("textarea"); - textarea.value = textToCopy; - textarea.style.position = "fixed"; - textarea.style.left = "-9999px"; - document.body.appendChild(textarea); - - try { - textarea.select(); - document.execCommand("copy"); - this.classList.add("copied"); - const icon = this.querySelector("i"); - icon.classList.remove("fa-copy"); - icon.classList.add("fa-check"); - - setTimeout(() => { - this.classList.remove("copied"); - icon.classList.remove("fa-check"); - icon.classList.add("fa-copy"); - }, 2000); - } catch (err) { - console.error("Copy failed:", err); - } finally { - document.body.removeChild(textarea); - } - }); + button.addEventListener("click", function() { + window.copyEmail(this); + }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
_team/index.md(8 hunks)assets/css/team.css(3 hunks)assets/js/main.js(1 hunks)
🧰 Additional context used
🪛 LanguageTool
_team/index.md
[uncategorized] ~9-~9: Did you mean “I”?
Context: ...ck;">](https://github.com/VatsalSy) [<i class="ai ai-google-scholar-square" sty...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~16-~16: Did you mean “I”?
Context: ...s - Univ. Twente / 2018-22, Thesis: <i class="fa-solid fa-file-pdf" style="mar...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~17-~17: Did you mean “I”?
Context: ...Lab, IIT-Roorkee / 2013-18, Thesis: <i class="fa-solid fa-file-pdf" style="mar...
(I_LOWERCASE_PREMIUM)
[inconsistency] ~24-~24: The abbreviation is usually written “Ph.D.”.
Context: ..._CV.pdf){: .cv-button} ## Ayush Dixit (Ph.D) [<i class="fa-brands fa-github" style=...
(PH_D)
[uncategorized] ~33-~33: Did you mean “I”?
Context: ...ity Lab, IIT-Roorkee / 2018-23,Thesis:<i class="fa-solid fa-file-pdf" style="mar...
(I_LOWERCASE_PREMIUM)
[inconsistency] ~37-~37: The abbreviation is usually written “Ph.D.”.
Context: ..., Respiratory Drops. ## Aman Bhargava (Ph.D) [<i class="fa-brands fa-github" style=...
(PH_D)
[uncategorized] ~299-~299: Did you mean “I”?
Context: ... with B.Sc., Univ. Twente - Thesis: <i class="fa-solid fa-file-pdf" style="mar...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~306-~306: Did you mean “I”?
Context: ...th M.Sc., Univ. Amsterdam - Thesis: <i class="fa-solid fa-file-pdf" style="mar...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~313-~313: Did you mean “I”?
Context: ...e.nl/people/profile/2119) - Thesis: <i class="fa-solid fa-file-pdf" style="mar...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~320-~320: Did you mean “I”?
Context: ...e.nl/people/profile/2064) - Thesis: <i class="fa-solid fa-file-pdf" style="mar...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~325-~325: Did you mean “I”?
Context: ...e.nl/people/profile/1741) - Thesis: <i class="fa-solid fa-file-pdf" style="mar...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~331-~331: Did you mean “I”?
Context: ...e.nl/people/profile/1741) - Thesis: <i class="fa-solid fa-file-pdf" style="mar...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~338-~338: Did you mean “I”?
Context: ...e.nl/people/profile/1822) - Thesis: <i class="fa-solid fa-file-pdf" style="mar...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~345-~345: Did you mean “I”?
Context: ... with B.Sc., Univ. Twente - Thesis: <i class="fa-solid fa-file-pdf" style="mar...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~352-~352: Did you mean “I”?
Context: ...e.nl/people/profile/1630) - Thesis: <i class="fa-solid fa-file-pdf" style="mar...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~358-~358: Did you mean “I”?
Context: ...e.nl/people/profile/1709) - Thesis: <i class="fa-solid fa-file-pdf" style="mar...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~365-~365: Did you mean “I”?
Context: ...e.nl/people/profile/1431) - Thesis: <i class="fa-solid fa-file-pdf" style="mar...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~380-~380: Did you mean “I”?
Context: ...e.nl/people/profile/1518) - Thesis: <i class="fa-solid fa-file-pdf" style="mar...
(I_LOWERCASE_PREMIUM)
🪛 Biome (1.9.4)
assets/js/main.js
[error] 6-6: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
[error] 117-117: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 251-251: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (7)
assets/css/team.css (5)
1-46: Base theme variables look solid
The new CSS variables under:rootcover a comprehensive palette and spacing scale, aligning with the project’s theming guidelines. No conflicts or overrides detected.
49-62: Dark theme overrides are correctly scoped
Your[data-theme="dark"]block cleanly overrides only the necessary variables. This follows the mobile-first and theming rules from the new CSS style guide.
78-86: Separated margin properties for.s-team__desc
Good job removing the shorthand that previously overrodemargin-bottomand instead using explicitmargin-left,margin-right, andmargin-bottom. This preserves the intended vertical spacing.
397-404: Consolidated.cv-buttonmargin shorthand correctly
The singlemargin: 1rem auto 1rem;now replaces multiple conflicting declarations, centering the button and preserving the vertical spacing.
475-485: Map section base styles are well-structured
The new.s-mapcontainer and row/column flex setup provide a clear, responsive foundation. They integrate seamlessly with the existing CSS variable system.assets/js/main.js (2)
63-73: History button creation is now properly protected.Good fix! You've addressed the previous review comment by only creating and appending the history button if
newsContentexists, preventing potentialTypeErrorif the element is missing.
229-232: User-friendly error message improvement.Great improvement! You've replaced the technical error message with a user-friendly one that doesn't expose implementation details, addressing the previous review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
assets/js/main.js (2)
246-249: Good improvement to user-facing error message.The error message has been improved to be more user-friendly, avoiding technical details that would confuse end users.
68-77: 🛠️ Refactor suggestionAdd more robust checking for newsContent element.
The history button creation and its attributes are defined outside the conditional check for newsContent. While the button is only appended if newsContent exists (line 87), it would be better to move the entire button creation inside that check to avoid unnecessary operations when the element doesn't exist.
-const historyBtn = document.createElement("a"); -historyBtn.href = "/history"; -historyBtn.className = "s-news__history-btn"; -historyBtn.innerHTML = - '<i class="fa-solid fa-arrow-right" style="margin-right: 8px; font-size: 1.2em;"></i>Archive'; -historyBtn.setAttribute("role", "button"); -historyBtn.setAttribute("tabindex", "0"); -historyBtn.setAttribute("aria-label", "View archive of all news items"); // Add keyboard event handler for accessibility -historyBtn.addEventListener("keydown", function(event) { - // Check for Enter (13) or Space (32) key - if (event.key === "Enter" || event.key === " " || event.keyCode === 13 || event.keyCode === 32) { - event.preventDefault(); - window.location.href = this.href; - } -}); if (newsContent) { + const historyBtn = document.createElement("a"); + historyBtn.href = "/history"; + historyBtn.className = "s-news__history-btn"; + historyBtn.innerHTML = + '<i class="fa-solid fa-arrow-right" style="margin-right: 8px; font-size: 1.2em;"></i>Archive'; + historyBtn.setAttribute("role", "button"); + historyBtn.setAttribute("tabindex", "0"); + historyBtn.setAttribute("aria-label", "View archive of all news items"); + + // Add keyboard event handler for accessibility + historyBtn.addEventListener("keydown", function(event) { + // Check for Enter (13) or Space (32) key + if (event.key === "Enter" || event.key === " " || event.keyCode === 13 || event.keyCode === 32) { + event.preventDefault(); + window.location.href = this.href; + } + }); newsContent.appendChild(historyBtn); }assets/css/team.css (2)
415-416: Fix margin shorthand property overriding margin-top.You've consolidated the margin values into a single declaration which is good practice. This fixes the previous issue where a margin shorthand was overriding an earlier margin-top declaration.
78-86: Fix margin shorthand overriding margin-bottom.You've properly fixed the previous issue where a margin shorthand was overriding the margin-bottom declaration by separating the margin properties into individual margin-left, margin-right, and margin-bottom.
🧹 Nitpick comments (8)
assets/js/main.js (3)
6-6: Remove redundant "use strict" directive.Since JavaScript modules and IIFE contexts already run in strict mode, this explicit directive is unnecessary.
- "use strict";🧰 Tools
🪛 Biome (1.9.4)
[error] 6-6: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
79-84: Remove deprecated keyCode usage.The
keyCodeproperty is deprecated in favor of using thekeyproperty. Since you're already checking for thekeyvalue, the additionalkeyCodechecks are unnecessary.historyBtn.addEventListener("keydown", function(event) { // Check for Enter (13) or Space (32) key - if (event.key === "Enter" || event.key === " " || event.keyCode === 13 || event.keyCode === 32) { + if (event.key === "Enter" || event.key === " ") { event.preventDefault(); window.location.href = this.href; } });
90-92: Add user-facing error message for news content failure.Similar to how you handle errors for featured papers (lines 246-249), consider adding a visible error message when news content fails to load, rather than just logging to console.
} catch (error) { console.error("Error loading news content:", error); + if (document.getElementById("news-section")) { + const errorMsg = document.createElement("div"); + errorMsg.className = "news-error"; + errorMsg.innerHTML = "<p>We're having trouble loading the latest news. Please try refreshing the page.</p>"; + document.getElementById("news-section").appendChild(errorMsg); + } }assets/css/team.css (1)
529-543: Consider using more specific selectors for overriding headings.While the multiple selectors with
!importantensure the heading styles are overridden, this approach could make future maintenance challenging. Consider using more specific selectors with the BEM methodology to avoid needing so many!importantdeclarations._team/index.md (4)
16-17: Improved thesis links with consistent styling.The updates to the thesis links follow a consistent format with PDF icons, making them more visually distinct and user-friendly. Make sure to update all inline styles to use the new
pdf-link-iconclass you've defined in your CSS.- <a href="https://doi.org/10.3990/1.9789036554077" class="pdf-link" style="display: inline-flex; align-items: center; margin-left: 5px;"><i class="fa-solid fa-file-pdf pdf-link-icon"></i>Viscous Free-Surface Flows</a> + <a href="https://doi.org/10.3990/1.9789036554077" class="pdf-link"><i class="fa-solid fa-file-pdf pdf-link-icon"></i>Viscous Free-Surface Flows</a>🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: Did you mean “I”?
Context: ...s - Univ. Twente / 2018-22, Thesis: <i class="fa-solid fa-file-pdf pdf-link-ic...(I_LOWERCASE_PREMIUM)
[uncategorized] ~17-~17: Did you mean “I”?
Context: ...Lab, IIT-Roorkee / 2013-18, Thesis: <i class="fa-solid fa-file-pdf pdf-link-ic...(I_LOWERCASE_PREMIUM)
299-299: Standardize thesis link styling for Milan Sent.For consistency with the other thesis links, remove the inline styles from the PDF link, as these styles should be defined in the CSS.
- <a href="https://tinyurl.com/2ycunjcr" class="pdf-link" style="display: inline-flex; align-items: center;"><i class="fa-solid fa-file-pdf pdf-link-icon"></i>Spinning Pizza</a> + <a href="https://tinyurl.com/2ycunjcr" class="pdf-link"><i class="fa-solid fa-file-pdf pdf-link-icon"></i>Spinning Pizza</a>🧰 Tools
🪛 LanguageTool
[uncategorized] ~299-~299: Did you mean “I”?
Context: ... with B.Sc., Univ. Twente - Thesis: <i class="fa-solid fa-file-pdf pdf-link-ic...(I_LOWERCASE_PREMIUM)
33-33: Fix spacing in thesis text.There should be spaces after punctuation and before the link.
- B.Tech + M.Tech, Two-Phase Flow & Instability Lab, IIT-Roorkee / 2018-23,**Thesis:**<a href="http://dx.doi.org/10.13140/RG.2.2.23218.79040" class="pdf-link" style="display: inline-flex; align-items: center; margin-left: 5px;"><i class="fa-solid fa-file-pdf pdf-link-icon"></i>Understanding interfacial interaction in macro and nano scales</a> + B.Tech + M.Tech, Two-Phase Flow & Instability Lab, IIT-Roorkee / 2018-23, **Thesis:** <a href="http://dx.doi.org/10.13140/RG.2.2.23218.79040" class="pdf-link" style="display: inline-flex; align-items: center; margin-left: 5px;"><i class="fa-solid fa-file-pdf pdf-link-icon"></i>Understanding interfacial interaction in macro and nano scales</a>🧰 Tools
🪛 LanguageTool
[uncategorized] ~33-~33: Did you mean “I”?
Context: ...ity Lab, IIT-Roorkee / 2018-23,Thesis:<i class="fa-solid fa-file-pdf pdf-link-ic...(I_LOWERCASE_PREMIUM)
24-24: Standardize Ph.D. abbreviation.For consistency, standardize the Ph.D. abbreviation with periods (Ph.D.) as this is the typical academic format and is used elsewhere in the document.
- ## Ayush Dixit (Ph.D) + ## Ayush Dixit (Ph.D.) - ## Aman Bhargava (Ph.D) + ## Aman Bhargava (Ph.D.)Also applies to: 37-37
🧰 Tools
🪛 LanguageTool
[inconsistency] ~24-~24: The abbreviation is usually written “Ph.D.”.
Context: ..._CV.pdf){: .cv-button} ## Ayush Dixit (Ph.D) [<i class="fa-brands fa-github" style=...(PH_D)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
_layouts/default.html(5 hunks)_team/index.md(8 hunks)assets/css/team.css(3 hunks)assets/js/main.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- _layouts/default.html
🧰 Additional context used
🪛 LanguageTool
_team/index.md
[uncategorized] ~9-~9: Did you mean “I”?
Context: ...ck;">](https://github.com/VatsalSy) [<i class="ai ai-google-scholar-square" sty...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~16-~16: Did you mean “I”?
Context: ...s - Univ. Twente / 2018-22, Thesis: <i class="fa-solid fa-file-pdf pdf-link-ic...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~17-~17: Did you mean “I”?
Context: ...Lab, IIT-Roorkee / 2013-18, Thesis: <i class="fa-solid fa-file-pdf pdf-link-ic...
(I_LOWERCASE_PREMIUM)
[inconsistency] ~24-~24: The abbreviation is usually written “Ph.D.”.
Context: ..._CV.pdf){: .cv-button} ## Ayush Dixit (Ph.D) [<i class="fa-brands fa-github" style=...
(PH_D)
[uncategorized] ~33-~33: Did you mean “I”?
Context: ...ity Lab, IIT-Roorkee / 2018-23,Thesis:<i class="fa-solid fa-file-pdf pdf-link-ic...
(I_LOWERCASE_PREMIUM)
[inconsistency] ~37-~37: The abbreviation is usually written “Ph.D.”.
Context: ..., Respiratory Drops. ## Aman Bhargava (Ph.D) [<i class="fa-brands fa-github" style=...
(PH_D)
[uncategorized] ~299-~299: Did you mean “I”?
Context: ... with B.Sc., Univ. Twente - Thesis: <i class="fa-solid fa-file-pdf pdf-link-ic...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~306-~306: Did you mean “I”?
Context: ...th M.Sc., Univ. Amsterdam - Thesis: <i class="fa-solid fa-file-pdf pdf-link-ic...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~313-~313: Did you mean “I”?
Context: ...e.nl/people/profile/2119) - Thesis: <i class="fa-solid fa-file-pdf pdf-link-ic...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~320-~320: Did you mean “I”?
Context: ...e.nl/people/profile/2064) - Thesis: <i class="fa-solid fa-file-pdf pdf-link-ic...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~325-~325: Did you mean “I”?
Context: ...e.nl/people/profile/1741) - Thesis: <i class="fa-solid fa-file-pdf pdf-link-ic...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~331-~331: Did you mean “I”?
Context: ...e.nl/people/profile/1741) - Thesis: <i class="fa-solid fa-file-pdf pdf-link-ic...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~338-~338: Did you mean “I”?
Context: ...e.nl/people/profile/1822) - Thesis: <i class="fa-solid fa-file-pdf pdf-link-ic...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~345-~345: Did you mean “I”?
Context: ... with B.Sc., Univ. Twente - Thesis: <i class="fa-solid fa-file-pdf pdf-link-ic...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~352-~352: Did you mean “I”?
Context: ...e.nl/people/profile/1630) - Thesis: <i class="fa-solid fa-file-pdf pdf-link-ic...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~358-~358: Did you mean “I”?
Context: ...e.nl/people/profile/1709) - Thesis: <i class="fa-solid fa-file-pdf pdf-link-ic...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~365-~365: Did you mean “I”?
Context: ...e.nl/people/profile/1431) - Thesis: <i class="fa-solid fa-file-pdf pdf-link-ic...
(I_LOWERCASE_PREMIUM)
[uncategorized] ~380-~380: Did you mean “I”?
Context: ...e.nl/people/profile/1518) - Thesis: <i class="fa-solid fa-file-pdf pdf-link-ic...
(I_LOWERCASE_PREMIUM)
🪛 Biome (1.9.4)
assets/js/main.js
[error] 6-6: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
[error] 133-133: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 267-267: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (5)
assets/js/main.js (2)
31-36: Good implementation of DOMPurify sanitization.You've correctly implemented content sanitization for the markdown parser to prevent XSS attacks. The proper sequence (parse, sanitize, then insert into DOM) is an important security improvement.
55-67: Good implementation of content sanitization and style fixes for list items.The code properly sanitizes the news content and adds style fixes for list item paragraphs to ensure correct display.
assets/css/team.css (2)
1-62: Well-structured CSS variables for theme management.Excellent work organizing the CSS variables into logical groups with clear naming conventions. The base theme variables and dark theme overrides provide a solid foundation for consistent styling across the site.
782-793: Good responsive design considerations for mobile devices.The media query for small screens (max-width: 500px) properly adjusts icon and button sizes for better usability on mobile devices.
_team/index.md (1)
60-60: Consistent date formatting for Ph.D. start dates.Good update to use consistent date formatting (Jun 2025) for both Ph.D. students' start dates.
Also applies to: 73-73
1 participant
historycollection from config