Skip to content

Conversation

SaptarshiMondal123
Copy link

@SaptarshiMondal123 SaptarshiMondal123 commented Sep 1, 2025

User description

I developed a fully responsive, dark-mode aware CSS template for Julep, designed to provide a clean and modern look for changelogs, blog entries, and documentation pages. This template uses CSS variables to simplify theme customization, supports syntax highlighting for code, styled inputs, tables, blockquotes, and tag badges, and ensures readability across devices. The dark-mode integration improves user experience for low-light environments, making the content easy on the eyes while maintaining visual hierarchy and accessibility.


PR Type

Enhancement


Description

  • Implement dark mode support with CSS variables

  • Simplify and modernize CSS template structure

  • Optimize chat widget theme detection logic

  • Remove deprecated styling and layout code


Diagram Walkthrough

flowchart LR
  A["CSS Variables"] --> B["Dark Mode Support"]
  C["Template Cleanup"] --> D["Modern Styling"]
  E["Widget Optimization"] --> F["Theme Detection"]
  B --> G["Enhanced UX"]
  D --> G
  F --> G
Loading

File Walkthrough

Relevant files
Enhancement
style.css
Complete CSS template modernization with dark mode             

changelog/Templates/index-template/style.css

  • Add CSS variables for light/dark theme support
  • Implement @media (prefers-color-scheme: dark) queries
  • Remove 285+ lines of legacy layout and styling code
  • Modernize base styles with variable-based theming
+76/-361
julep-chat-widget.js
Streamline chat widget theme detection                                     

documentation/scripts/julep-chat-widget.js

  • Simplify theme detection logic and MutationObserver setup
  • Remove system theme change listener
  • Optimize DOM ready state handling
+9/-15   

Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Dark Hover Contrast

In dark mode, the link hover background uses a fixed rgba(0,0,0,0.05) which may be imperceptible or reduce contrast against the dark background; consider a theme-aware hover background variable.

a:hover { background: rgba(0,0,0,0.05); color: var(--link-hover-color); }
a:active { opacity: 0.9; }
Theming Consistency

Some colors like stable/dev tag colors are duplicated in both light and dark variables without adjustment; verify they meet contrast requirements on dark backgrounds and consider deriving them from variables or adjusting per theme.

--stable-green: #28a745;
--dev-orange: #d49144;
--tag-bg-main: #28a745;
--tag-bg-dev: #d49144;
--footer-color: #bbb;
--input-bg: #2a2a2a;
Theme Observer Scope

MutationObserver now only tracks class changes on ; if sites use data-theme or other attributes, theme sync may break. Confirm consumers only toggle classes or broaden the observer as needed.

const observer = new MutationObserver(() => detectTheme());
observer.observe(document.documentElement, { 
  attributes: true, 
  attributeFilter: ['class'] 
});

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Caution

Changes requested ❌

Reviewed everything up to 3d9c11a in 1 minute and 41 seconds. Click for details.
  • Reviewed 513 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. documentation/scripts/julep-chat-widget.js:792
  • Draft comment:
    The MutationObserver now only watches the 'class' attribute. If themes are toggled via a 'data-theme' attribute elsewhere, the theme detection might not trigger.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
2. documentation/scripts/julep-chat-widget.js:800
  • Draft comment:
    Initialization now calls init() directly instead of using setTimeout. Confirm that this change doesn’t introduce any race conditions on pages with slower DOM readiness.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to confirm that a change doesn't introduce race conditions, which is against the rules. It doesn't provide a specific suggestion or point out a clear issue with the code.
3. changelog/Templates/index-template/style.css:4
  • Draft comment:
    Typo: The selector on line 4 uses a double colon (::root) instead of a single colon (:root). Please fix the selector to use a single colon.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.

Workflow ID: wflow_G6pl2ZObxOfsDbsS

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Restore critical legacy styles and triggers

The PR drops core selectors (tables/lists/images, .hljs syntax tokens, and
layout/index classes like .wide/.margin/a.row/hr.full) without replacements,
which will break existing content and code highlighting. Reintroduce
variable-based equivalents for these critical styles and make the stylesheet
respond to an html.dark class (not just prefers-color-scheme) so the page stays
in sync with the widget’s theme detection. Additionally, retain a system theme
change fallback (or class-based alternative) in the widget to avoid regressions
on sites that don’t toggle the html class.

Examples:

changelog/Templates/index-template/style.css [4-105]
:root {
  --body-bg: #ffffff;
  --text-color: #333333;
  --link-color: #0077cc;
  --link-hover-color: #468cc7;
  --border-color: #eee;
  --stable-green: #28a745;
  --dev-orange: #d49144;
  --tag-bg-main: #28a745;
  --tag-bg-dev: #d49144;

 ... (clipped 92 lines)
documentation/scripts/julep-chat-widget.js [757-803]
  function detectTheme() {
    // Check for Mintlify's theme - they use 'dark' class on html element
    const htmlElement = document.documentElement;
    const isDark = htmlElement.classList.contains('dark');
    
    // Apply theme to widget
    widget.setAttribute('data-theme', isDark ? 'dark' : 'light');
    console.log('[Julep Widget] Theme detected:', isDark ? 'dark' : 'light');
    
    // Debug: log what we're checking

 ... (clipped 37 lines)

Solution Walkthrough:

Before:

/* style.css */
@media (prefers-color-scheme: dark) {
  :root {
    /* dark theme variables */
  }
}
/* Styles for tables, lists, .hljs, .wide, etc. are removed. */

/* julep-chat-widget.js */
function detectTheme() {
  const isDark = document.documentElement.classList.contains('dark');
  widget.setAttribute('data-theme', isDark ? 'dark' : 'light');
}

function init() {
  detectTheme();
  // Only observes html class for changes.
  const observer = new MutationObserver(() => detectTheme());
  observer.observe(document.documentElement, { attributeFilter: ['class'] });
}

After:

/* style.css */
html.dark:root,
@media (prefers-color-scheme: dark) {
  :root {
    /* dark theme variables */
  }
}
/* Re-add critical styles using variables */
table { border-color: var(--border-color); }
.hljs { background: var(--code-bg); }
/* ... etc. */

/* julep-chat-widget.js */
function detectTheme() {
  const siteIsDark = document.documentElement.classList.contains('dark');
  const systemIsDark = window.matchMedia('(prefers-color-scheme: dark)').matches;
  const isDark = siteIsDark || systemIsDark;
  widget.setAttribute('data-theme', isDark ? 'dark' : 'light');
}

function init() {
  detectTheme();
  // Observe both html class and system theme
  const observer = new MutationObserver(detectTheme);
  observer.observe(document.documentElement, { attributeFilter: ['class'] });
  window.matchMedia('(prefers-color-scheme: dark)').addEventListener('change', detectTheme);
}
Suggestion importance[1-10]: 10

__

Why: The suggestion correctly identifies multiple critical issues, including the removal of essential styles for tables, lists, and .hljs syntax highlighting, and a theme detection mismatch between the page and the widget, which would break functionality and user experience.

High
General
Handle OS theme changes

Re-introduce a matchMedia('(prefers-color-scheme: dark)') listener so the widget
tracks OS-level theme switches when the page uses only media queries and doesn’t
mutate DOM attributes. Include a legacy fallback for older browsers.

documentation/scripts/julep-chat-widget.js [790-795]

 // Watch for theme changes (Mintlify toggles `dark` on <html>)
 const observer = new MutationObserver(() => detectTheme());
 observer.observe(document.documentElement, { 
   attributes: true, 
   attributeFilter: ['class'] 
 });
 
+// Also watch for system theme changes
+const media = window.matchMedia('(prefers-color-scheme: dark)');
+if (typeof media.addEventListener === 'function') {
+  media.addEventListener('change', detectTheme);
+} else if (typeof media.addListener === 'function') {
+  media.addListener(detectTheme);
+}
+
  • Apply / Chat
Suggestion importance[1-10]: 10

__

Why: This suggestion is critical because it fixes a regression introduced in the PR; it restores the listener for prefers-color-scheme, which is the exact mechanism the new CSS in this PR uses for theming.

High
Possible issue
Broaden theme detection sources

Make theme detection robust by also checking the data-theme attribute and the
body element. Some setups toggle data-theme="dark" or apply the dark class on
body, and the widget will otherwise fail to sync.

documentation/scripts/julep-chat-widget.js [757-768]

 function detectTheme() {
-  // Check for Mintlify's theme - they use 'dark' class on html element
-  const htmlElement = document.documentElement;
-  const isDark = htmlElement.classList.contains('dark');
+  const html = document.documentElement;
+  const body = document.body;
+  const attrTheme = (html.getAttribute('data-theme') || (body && body.getAttribute('data-theme')) || '').toLowerCase();
+  const isDark =
+    html.classList.contains('dark') ||
+    (body && body.classList.contains('dark')) ||
+    attrTheme === 'dark';
   
-  // Apply theme to widget
   widget.setAttribute('data-theme', isDark ? 'dark' : 'light');
   console.log('[Julep Widget] Theme detected:', isDark ? 'dark' : 'light');
-  
-  // Debug: log what we're checking
-  console.log('[Julep Widget] HTML classes:', htmlElement.className);
+  console.log('[Julep Widget] HTML classes:', html.className);
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion improves the widget's robustness by checking for theme settings on the body element and via the data-theme attribute, making it compatible with more website implementations.

Medium
Observe html and body for changes

Observe both html and body and include data-theme in the attribute filter. This
ensures the widget updates when either element changes class or data-theme.

documentation/scripts/julep-chat-widget.js [790-795]

-// Watch for theme changes (Mintlify toggles `dark` on <html>)
+// Watch for theme changes (Mintlify toggles `dark` on <html> or updates data-theme)
 const observer = new MutationObserver(() => detectTheme());
-observer.observe(document.documentElement, { 
-  attributes: true, 
-  attributeFilter: ['class'] 
-});
+const config = { attributes: true, attributeFilter: ['class', 'data-theme'] };
+observer.observe(document.documentElement, config);
+if (document.body) observer.observe(document.body, config);
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: This suggestion correctly proposes to expand the MutationObserver to watch for data-theme and body element changes, which is necessary for the theme detection to be robust across different sites.

Medium
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant