Repository: mlflow/mlflow
Pull request: Improve DarkThemeSwitch UX: Remove switch and make icon clickable
This PR improves the user experience of the `DarkThemeSwitch` component by removing the redundant switch toggle and making the theme icon directly clickable.

## Changes Made

### Before
```tsx
<Switch
  componentId="..."
  checked={isDarkTheme}
  onChange={setIsDarkTheme}
/>
{isDarkTheme ? <MoonIcon /> : <SunIcon />}
```

### After
```tsx
<ToggleIconButton
  componentId="..."
  pressed={isDarkTheme}
  onClick={() => setIsDarkTheme(!isDarkTheme)}
  icon={isDarkTheme ? <MoonIcon /> : <SunIcon />}
  aria-label={isDarkTheme ? 'Switch to light theme' : 'Switch to dark theme'}
/>
```

(haru posted this, not copilot)

https://github.com/user-attachments/assets/4a458f63-a206-45fc-b8e3-a81c4d30a959

## Benefits

- **Cleaner UI**: Single interactive element instead of two separate components
- **Better UX**: More intuitive - users expect to click the theme icon directly
- **Improved Accessibility**: Added proper ARIA labels that dynamically change based on current theme
- **Consistency**: Aligns with common theme toggle patterns in modern web applications
- **Reduced Complexity**: Fewer components to maintain and test

## Technical Details

- Replaced `@databricks/design-system` Switch with existing `ToggleIconButton` component
- Maintained original visual design and spacing
- Added dynamic accessibility labels for screen readers
- Preserved existing props interface for backward compatibility
- Included hover/focus states automatically via `ToggleIconButton`

## Testing

- Added comprehensive test suite covering all functionality
- Verified accessibility with keyboard navigation tests
- Ensured proper ARIA label updates based on theme state
- All existing functionality preserved

## Files Changed

- `mlflow/server/js/src/common/components/DarkThemeSwitch.tsx` - Updated component implementation
- `mlflow/server/js/src/common/components/DarkThemeSwitch.test.tsx` - Added test coverage

Fixes #16542.

<!-- START COPILOT CODING AGENT TIPS -->
---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs.

Problem statement:
Improve DarkThemeSwitch UX: Remove switch and make icon clickable
## Summary

Currently, the `DarkThemeSwitch` component in `mlflow/server/js/src/common/components/DarkThemeSwitch.tsx` has both a switch toggle and an icon (moon/sun). This creates redundant UI elements for the same functionality.

## Proposed Changes

1. **Remove the Switch component** - Currently using `@databricks/design-system` Switch
2. **Make the icon clickable** - Transform the moon/sun icon into a clickable button
3. **Improve accessibility** - Add proper ARIA labels and keyboard navigation support

## Current Implementation

```tsx
<Switch
  componentId="codegen_mlflow_app_src_common_components_darkthemeswitch.tsx_32"
  checked={isDarkTheme}
  onChange={setIsDarkTheme}
/>
{isDarkTheme ? <MoonIcon /> : <SunIcon />}
```

## Suggested Implementation

The icon should become a clickable button that:
- Toggles the theme when clicked
- Has proper hover/focus states
- Includes accessibility attributes (aria-label, role)
- Maintains the current visual design with moon/sun icons

## Benefits

- **Cleaner UI**: Single interactive element instead of two
- **Better UX**: More intuitive - users expect to click the theme icon
- **Consistency**: Aligns with common theme toggle patterns in web applications
- **Reduced complexity**: Less components to maintain

## Files to Modify

- `mlflow/server/js/src/common/components/DarkThemeSwitch.tsx`

## Acceptance Criteria

- [ ] Switch component is removed
- [ ] Moon/Sun icon is clickable and toggles theme
- [ ] Proper hover/focus states are implemented
- [ ] Accessibility attributes are added
- [ ] Visual spacing and alignment are maintained
- [ ] Component continues to work with existing props interface

Hints from issue discussion:

Commit message: Fix DarkThemeSwitch alignment with GitHub/Docs links by reducing button size

Co-authored-by: harupy <17039389+harupy@users.noreply.github.com>

Patch to review:
diff --git a/mlflow/server/js/src/common/components/DarkThemeSwitch.test.tsx b/mlflow/server/js/src/common/components/DarkThemeSwitch.test.tsx
new file mode 100644
index 0000000000000..b76f0059d9a2e
--- /dev/null
+++ b/mlflow/server/js/src/common/components/DarkThemeSwitch.test.tsx
@@ -0,0 +1,56 @@
+import React from 'react';
+import { renderWithIntl, screen } from '@mlflow/mlflow/src/common/utils/TestUtils.react18';
+import { DarkThemeSwitch } from './DarkThemeSwitch';
+import userEvent from '@testing-library/user-event';
+
+describe('DarkThemeSwitch', () => {
+  const mockSetIsDarkTheme = jest.fn();
+
+  beforeEach(() => {
+    mockSetIsDarkTheme.mockClear();
+  });
+
+  test('should render toggle button with sun icon when light theme is active', () => {
+    renderWithIntl(<DarkThemeSwitch isDarkTheme={false} setIsDarkTheme={mockSetIsDarkTheme} />);
+
+    const button = screen.getByRole('button');
+    expect(button).toBeInTheDocument();
+    expect(button).toHaveAttribute('aria-label', 'Switch to dark theme');
+  });
+
+  test('should render toggle button with moon icon when dark theme is active', () => {
+    renderWithIntl(<DarkThemeSwitch isDarkTheme setIsDarkTheme={mockSetIsDarkTheme} />);
+
+    const button = screen.getByRole('button');
+    expect(button).toBeInTheDocument();
+    expect(button).toHaveAttribute('aria-label', 'Switch to light theme');
+  });
+
+  test('should call setIsDarkTheme with opposite value when clicked', async () => {
+    renderWithIntl(<DarkThemeSwitch isDarkTheme={false} setIsDarkTheme={mockSetIsDarkTheme} />);
+
+    const button = screen.getByRole('button');
+    await userEvent.click(button);
+
+    expect(mockSetIsDarkTheme).toHaveBeenCalledWith(true);
+  });
+
+  test('should call setIsDarkTheme with opposite value when clicked in dark mode', async () => {
+    renderWithIntl(<DarkThemeSwitch isDarkTheme setIsDarkTheme={mockSetIsDarkTheme} />);
+
+    const button = screen.getByRole('button');
+    await userEvent.click(button);
+
+    expect(mockSetIsDarkTheme).toHaveBeenCalledWith(false);
+  });
+
+  test('should be keyboard accessible', async () => {
+    renderWithIntl(<DarkThemeSwitch isDarkTheme={false} setIsDarkTheme={mockSetIsDarkTheme} />);
+
+    const button = screen.getByRole('button');
+    button.focus();
+
+    await userEvent.keyboard('{Enter}');
+    expect(mockSetIsDarkTheme).toHaveBeenCalledWith(true);
+  });
+});
diff --git a/mlflow/server/js/src/common/components/DarkThemeSwitch.tsx b/mlflow/server/js/src/common/components/DarkThemeSwitch.tsx
index f83b2ed65d53d..2e156dc20242d 100644
--- a/mlflow/server/js/src/common/components/DarkThemeSwitch.tsx
+++ b/mlflow/server/js/src/common/components/DarkThemeSwitch.tsx
@@ -1,6 +1,6 @@
 import React from 'react';
 
-import { Switch } from '@databricks/design-system';
+import { ToggleIconButton } from './ToggleIconButton';
 
 const MoonIcon = () => (
   <svg xmlns="http://www.w3.org/2000/svg" height="1em" viewBox="0 0 384 512" css={{ fill: 'currentcolor' }}>
@@ -23,17 +23,17 @@ export const DarkThemeSwitch = ({
   isDarkTheme: boolean;
   setIsDarkTheme: (isDarkTheme: boolean) => void;
 }) => (
-  <span
+  <ToggleIconButton
+    componentId="codegen_mlflow_app_src_common_components_darkthemeswitch.tsx_32"
+    pressed={isDarkTheme}
+    onClick={() => setIsDarkTheme(!isDarkTheme)}
+    icon={isDarkTheme ? <MoonIcon /> : <SunIcon />}
+    aria-label={isDarkTheme ? 'Switch to light theme' : 'Switch to dark theme'}
     css={{
-      display: 'flex',
-      paddingTop: 4,
+      background: 'transparent',
+      width: '1.5em',
+      height: '1.5em',
+      fontSize: '16px',
     }}
-  >
-    <Switch
-      componentId="codegen_mlflow_app_src_common_components_darkthemeswitch.tsx_32"
-      checked={isDarkTheme}
-      onChange={setIsDarkTheme}
-    />
-    {isDarkTheme ? <MoonIcon /> : <SunIcon />}
-  </span>
+  />
 );
diff --git a/mlflow/server/js/src/common/components/MlflowHeader.tsx b/mlflow/server/js/src/common/components/MlflowHeader.tsx
index 0a9fde953074e..8ab16b2f5b81b 100644
--- a/mlflow/server/js/src/common/components/MlflowHeader.tsx
+++ b/mlflow/server/js/src/common/components/MlflowHeader.tsx
@@ -65,7 +65,7 @@ export const MlflowHeader = ({
         </span>
       </div>
       <div css={{ flex: 1 }} />
-      <div css={{ display: 'flex', gap: theme.spacing.lg }}>
+      <div css={{ display: 'flex', gap: theme.spacing.lg, alignItems: 'center' }}>
         <DarkThemeSwitch isDarkTheme={isDarkTheme} setIsDarkTheme={setIsDarkTheme} />
         <a href="https://github.com/mlflow/mlflow">GitHub</a>
         <a href={HomePageDocsUrl}>Docs</a>


Code review comments:
diff_hunk: @@ -23,17 +23,17 @@ export const DarkThemeSwitch = ({
   isDarkTheme: boolean;
   setIsDarkTheme: (isDarkTheme: boolean) => void;
 }) => (
-  <span
+  <ToggleIconButton
+    componentId="codegen_mlflow_app_src_common_components_darkthemeswitch.tsx_32"
+    pressed={isDarkTheme}
+    onClick={() => setIsDarkTheme(!isDarkTheme)}
+    icon={isDarkTheme ? <MoonIcon /> : <SunIcon />}
+    aria-label={isDarkTheme ? 'Switch to light theme' : 'Switch to dark theme'}
     css={{
-      display: 'flex',
-      paddingTop: 4,
+      background: 'transparent',
+      width: '1.5em',
+      height: '1.5em',
+      fontSize: '16px',
path: mlflow/server/js/src/common/components/DarkThemeSwitch.tsx
text: @user1:
```suggestion
```

let's remove this