Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fluent: add dark theme support #5138

Merged
merged 4 commits into from
Apr 18, 2024
Merged

Fluent: add dark theme support #5138

merged 4 commits into from
Apr 18, 2024

Conversation

OEvgeny
Copy link
Collaborator

@OEvgeny OEvgeny commented Apr 17, 2024

Fixes #

Changelog Entry

  • (Experimental) Added botframework-webchat-fluent-theme package for applying Fluent UI theme to Web Chat, by @compulim and @OEvgeny
    • Added dark theme support, in PR #5138

Description

PR adds styles needed to support dark themes. Mostly overriding user-agent styles with our or Fluent defaults (when Fluent theme is provided).

Design

Note that the fluent-theme package doesn't inherit the styleOptions colors. Instead, it defaults to the provided by Fluent CSS variables.

Specific Changes

  • Fix missing colors in buttons rules
  • Remove redundant fill attribute from SVG elements
  • Add tests with dark Fluent theme applied
  • Fix attachments display if telephone keypad is opened
  • Add selected state for toolbar button
  • Align toolbar button styles with Figma

-

  • I have added tests and executed them locally
  • I have updated CHANGELOG.md
  • I have updated documentation

Review Checklist

This section is for contributors to review your work.

  • Accessibility reviewed (tab order, content readability, alt text, color contrast)
  • Browser and platform compatibilities reviewed
  • CSS styles reviewed (minimal rules, no z-index)
  • Documents reviewed (docs, samples, live demo)
  • Internationalization reviewed (strings, unit formatting)
  • package.json and package-lock.json reviewed
  • Security reviewed (no data URIs, check for nonce leak)
  • Tests reviewed (coverage, legitimacy)

@OEvgeny OEvgeny changed the title Fluent: add dark mode support Fluent: add dark theme support Apr 17, 2024
@OEvgeny OEvgeny marked this pull request as ready for review April 17, 2024 21:03
Copy link
Contributor

@compulim compulim left a comment

Choose a reason for hiding this comment

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

Could we try something real? I am unsure if this can be easily done because how Fluent package their stuff...

Let's see if you can do this. Load @fluentui/theme (from unpkg.com?) and render the actual theme provider before us. So we can see the E2E of this user story: "Web Chat theme can be using defaults from Fluent UI."

Then turn on the dark theme. I think Fluent component should honor that and set the CSS properly.

To turn on dark theme, you can search in some of our HTML tests. I forgot which HTML test, search "prefers-color-scheme". It could be related to keyboard help screen.

To turn on dark theme, you can look at this keyboard help screen test,
https://github.com/microsoft/BotFramework-WebChat/tree/main/__tests__/html/accessibility.keyboardHelp.darkTheme.html#L17.

Particularly, send setEmulatedMedia CDP command over WebDriver. 😉

await host.sendDevToolsCommand('Emulation.setEmulatedMedia', {
  features: [{ name: 'prefers-color-scheme', value: 'dark' }]
});

@OEvgeny
Copy link
Collaborator Author

OEvgeny commented Apr 18, 2024

Could we try something real? I am unsure if this can be easily done because how Fluent package their stuff...

Let's see if you can do this. Load @fluentui/theme (from unpkg.com?) and render the actual theme provider before us. So we can see the E2E of this user story: "Web Chat theme can be using defaults from Fluent UI."

Then turn on the dark theme. I think Fluent component should honor that and set the CSS properly.

To turn on dark theme, you can search in some of our HTML tests. I forgot which HTML test, search "prefers-color-scheme". It could be related to keyboard help screen.

To turn on dark theme, you can look at this keyboard help screen test, https://github.com/microsoft/BotFramework-WebChat/tree/main/__tests__/html/accessibility.keyboardHelp.darkTheme.html#L17.

Particularly, send setEmulatedMedia CDP command over WebDriver. 😉

await host.sendDevToolsCommand('Emulation.setEmulatedMedia', {
  features: [{ name: 'prefers-color-scheme', value: 'dark' }]
});

I was hoping to use the Fluent infrastructure too, but I haven't found a reliable way to bundle Fluent, unless we want to maintain the ESM build ourselves.

I'm blocked on @fluentui/theme importing react package. The following pattern works with simple things:

    <script type="importmap">
      {
        "imports": {
          "@fluentui/theme": "https://unpkg.com/@fluentui/[email protected]/lib/index.js",
          "@fluentui/merge-styles": "https://unpkg.com/@fluentui/[email protected]/lib/index.js",
          "@fluentui/utilities": "https://unpkg.com/@fluentui/[email protected]/lib/index.js",
          "@fluentui/dom-utilities": "https://unpkg.com/@fluentui/[email protected]/lib/index.js",
          "@fluentui/set-version": "https://unpkg.com/@fluentui/[email protected]/lib/index.js",
          "tslib": "https://unpkg.com/[email protected]/tslib.es6.mjs"
        }
      }
    </script>
    <script type="module">
      import('@fluentui/theme').then(mod => {
        console.log(mod);
      })
    </script>

And I probably can work around react not providing ESM builds, but I found that in order to apply the theme, I have to make the whole @fluentui/react-components library to work this way which is way more packages thus time figuring it out.

@compulim please let me know if you have a better idea, or if you think it's worth to continue.

@OEvgeny OEvgeny requested a review from compulim April 18, 2024 17:52
@compulim
Copy link
Contributor

No problems. I expect this outcome too.

After you finish all P0s, please try to build our own bundle with @fluentui/theme and react inside Web Chat monorepo. Feel free to put it under /packages/test/ or even under existing package. I think this will be a valuable test as we deliver what we promise. 😉

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.

2 participants