zendeskgarden / react-components

:seedling: garden React components
https://zendeskgarden.github.io/react-components/
Apache License 2.0
1.1k stars 89 forks source link

feat: applies color-scheme to align garden and system themes #1873

Closed geotrev closed 3 months ago

geotrev commented 3 months ago

Description

Currently, system controls (scrollbars) are forced to their default color even in Garden dark mode (normal).

This PR sets the color-scheme property throughout several components to ensure system controls switch between light and dark theme alongside Garden's theme.

Detail

To ensure the theme is retained regardless of system settings, the value is set using the only keyword (see spec).

To test, I include the following snippet inside the content of the affected component in dark theme (and then swap the value to dark when testing light theme).

<ThemeProvider theme={pt => ({ ...pt, colors: { ...pt.colors, base: 'light' } })}>
  Nested content
</ThemeProvider>

I then applied overflowY or overflowX with auto to containers that can be scrollable. To create constrain, maxHeight and maxWidth were added to either the component itself OR a containing div. These testing parameters help mimic real world scenarios where a consumer could reach for either option to achieve constrained content dimensions.

For example, compare direct component constrain (note Tabs and Tab.TabPanel):

<Tabs {...args} style={{ maxWidth: 250 }}>
  <Tabs.TabList style={{ overflowY: 'auto' }}>
    {tabs.map(tab => (
      <Tabs.Tab key={tab.value} item={tab.value} disabled={tab.disabled}>
        {tab.value}
      </Tabs.Tab>
    ))}
  </Tabs.TabList>
  {tabs.map(tab => (
    <Tabs.TabPanel key={tab.value} item={tab.value} style={{ overflowY: 'auto', maxHeight: 200 }}>
      {tab.panel}
    </Tabs.TabPanel>
  ))}
</Tabs>

With container-based constrain using div, meant to simulate page layout constraints:

<div style={{ maxWidth: 250, maxHeight: 300 }}>
  <Tabs {...args}>
    <Tabs.TabList style={{ overflowY: 'auto' }}>
      {tabs.map(tab => (
        <Tabs.Tab key={tab.value} item={tab.value} disabled={tab.disabled}>
          {tab.value}
        </Tabs.Tab>
      ))}
    </Tabs.TabList>
      <Tabs.TabPanel key={tab.value} item={tab.value} style={{ overflowY: 'auto' }}>
        {tab.panel}
      </Tabs.TabPanel>
  </Tabs>
</div>

List of components + screenshots below.

Chrome (Content + Sheet)

Screenshot 2024-07-25 at 10 12 43 AM

Grid

Screenshot 2024-07-25 at 10 10 17 AM

Modal

Screenshot 2024-07-25 at 10 08 43 AM

Drawer

Screenshot 2024-07-25 at 10 09 24 AM

TooltipModal

Screenshot 2024-07-25 at 10 08 24 AM

Menu

Screenshot 2024-07-25 at 10 11 46 AM

Combobox

Multiselectable scrolling

Screenshot 2024-07-25 at 2 40 29 PM

Listbox

Screenshot 2024-07-25 at 10 11 39 AM

Pane (PaneProvider)

Screenshot 2024-07-25 at 10 11 29 AM

Tabs (TabList + TabPanel)

Screenshot 2024-07-25 at 11 07 13 AM

Exclusions

Checklist

geotrev commented 3 months ago

Are we missing the Combobox trigger?

@jzempel Good catch, updated!

geotrev commented 3 months ago

I can wait to test Combobox when #1872 merges, assuming there are any differences in behavior.