[system] Missing quotes in `InitColorSchemeScript.tsx`

brandongit2 commented 2 months ago

Steps to reproduce

Link to live example: https://stackblitz.com/edit/github-xk9wmf?file=src%2FApp.tsx

import { FC } from 'react';
import InitColorSchemeScript from '@mui/material/InitColorSchemeScript';

import './style.css';

export const App: FC<{ name: string }> = ({ name }) => {
  // This injects a `<script>` tag into the DOM. Right-click and inspect the
  // site preview on the right, and look for the `<script>` tag under `<div id="app">`.
  // You should see that on the second-last line of the script, there is a bare `%s` with
  // no quotes around it. This throws a `SyntaxError` in my browser.
  return <InitColorSchemeScript attribute="[data-color-scheme=%s]" />;
};

Steps:

Right-click and inspect the site preview on the right, and look for the <script> tag under <div id="app">. You should see that on the second-last line of the script, there is a bare %s with no quotes around it. This throws a SyntaxError in my browser.

Current behavior

No response

Expected behavior

No response

Context

https://github.com/mui/material-ui/blob/9f4b846be96ee18733b6bd77eba2f54b1ceb9b04/packages/mui-system/src/InitColorSchemeScript/InitColorSchemeScript.tsx#L81

needs to have quotes around ${value}.

Your environment

System: OS: macOS 14.6.1 Binaries: Node: 18.19.1 - ~/.asdf/installs/nodejs/18.19.1/bin/node npm: 10.2.4 - ~/.asdf/plugins/nodejs/shims/npm pnpm: 9.7.1 - ~/.asdf/installs/nodejs/18.19.1/bin/pnpm Browsers: Chrome: 127.0.6533.120 Edge: Not Found Safari: 17.6 npmPackages: @emotion/react: 11.13.3 => 11.13.3 @emotion/styled: 11.13.0 => 11.13.0 @mui/icons-material: 6.0.0-rc.0 => 6.0.0-rc.0 @mui/lab: 6.0.0-beta.7 => 6.0.0-beta.7 @mui/material: 6.0.0-rc.0 => 6.0.0-rc.0 @mui/system: 6.0.0-rc.0 => 6.0.0-rc.0 @mui/x-data-grid: 7.14.0 => 7.14.0 @mui/x-date-pickers: 7.14.0 => 7.14.0 @mui/x-tree-view: 7.14.0 => 7.14.0 @types/react: 18.3.4 => 18.3.4 react: 18.3.1 => 18.3.1 react-dom: 18.3.1 => 18.3.1 typescript: ^5.2.2 => 5.3.2

Search keywords: css variables, InitColorSchemeScript, %s

siriwatknp commented 2 months ago

I think this is not a bug (may be a documentation improvement). You missed the single quote '%s':

-<InitColorSchemeScript attribute="[data-color-scheme=%s]" />
+<InitColorSchemeScript attribute="[data-color-scheme='%s']" />

oliviertassinari commented 2 months ago

+1 for solving this using the docs, I didn't find one in https://next.mui.com/material-ui/customization/css-theme-variables/configuration/.

Also there seems that we are missing the API page for the <InitColorSchemeScript> component.

@siriwatknp This isn't valid JavaScript though, it crashes, maybe there should be a defensive logic.

(function() {
try {
  var mode = localStorage.getItem('mui-mode') || 'system';
  var colorScheme = '';
  var dark = localStorage.getItem('mui-color-scheme-dark') || 'dark';
  var light = localStorage.getItem('mui-color-scheme-light') || 'light';
  if (mode === 'system') {
    // handle system mode
    var mql = window.matchMedia('(prefers-color-scheme: dark)');
    if (mql.matches) {
      colorScheme = dark
    } else {
      colorScheme = light
    }
  }
  if (mode === 'light') {
    colorScheme = light;
  }
  if (mode === 'dark') {
    colorScheme = dark;
  }
  if (colorScheme) {

      document.documentElement.setAttribute('data-color-scheme'.replace('%s', colorScheme), %s.replace('%s', colorScheme));
  }
} catch(e){}})();

brandongit2 commented 2 months ago

Ah okay. I was going off of the comment on this line: https://github.com/mui/material-ui/blob/9f4b846be96ee18733b6bd77eba2f54b1ceb9b04/packages/mui-system/src/InitColorSchemeScript/InitColorSchemeScript.tsx#L73

Also, @oliviertassinari it is mentioned in the docs page you linked, you just have to click on the little tabs here:

But yeah it took me quite a while before I found that. It would be nice if the docs around colorSchemeSelector and InitColorSchemeScript were improved.

oliviertassinari commented 2 months ago

you have to click on the little tabs here:

@brandongit2 Oh, I couldn't find it, thanks.

So if we summarily the next step, it looks like solving this issue is about the docs:

[ ] 1. Fix the code comment:


diff --git a/packages/mui-system/src/InitColorSchemeScript/InitColorSchemeScript.tsx b/packages/mui-system/src/InitColorSchemeScript/InitColorSchemeScript.tsx
index 7320bb7c10..c0f6eca59f 100644
--- a/packages/mui-system/src/InitColorSchemeScript/InitColorSchemeScript.tsx
+++ b/packages/mui-system/src/InitColorSchemeScript/InitColorSchemeScript.tsx
@@ -70,7 +70,7 @@ export default function InitColorSchemeScript(options?: InitColorSchemeScriptPro
 setter += `${colorSchemeNode}.classList.remove('${selector}'.replace('%s', light), '${selector}'.replace('%s', dark));
   ${colorSchemeNode}.classList.add('${selector}'.replace('%s', colorScheme));`;
}

const matches = attribute.match(/[([^]]+)]/); // case [data-color-scheme=%s] or [data-color-scheme]
const matches = attribute.match(/[([^]]+)]/); // case [data-color-scheme='%s'] or [data-color-scheme] if (matches) { const [attr, value] = matches[1].split('='); if (!value) {
[ ] 2. Create the API page for the InitColorSchemeScript component at https://mui.com/system/api/init-color-scheme-script/.
[ ] 3. Flatten the docs for the colorSchemeSelector. People shouldn't have to click to find meaningful docs content, it should be scrollable/searchable. For example https://tailwindcss.com/docs/dark-mode#customizing-the-selector this looks correct.
[ ] 4. Add some demos for InitColorSchemeScript.

mui / material-ui