Esri / calcite-design-system

A monorepo containing the packages for Esri's Calcite Design System
https://developers.arcgis.com/calcite-design-system/
Other
293 stars 76 forks source link

[Dialog] - content background should be white #10809

Open ashetland opened 5 days ago

ashetland commented 5 days ago

Check existing issues

Actual Behavior

The background of the content area is color.background.

Expected Behavior

The background is color.foreground.1.

Reproduction Sample

https://codepen.io/ashetland/pen/WNVqpjw?editors=100

Reproduction Steps

Compare the reproduction sample of Dialog with Modal.

Reproduction Version

v2.13

Relevant Info

cc @jcfranco @geospatialem

Regression?

No response

Priority impact

impact - p1 - need for current milestone

Impact

Modal was deprecated in favor of Dialog, but their background colors have different defaults. Correcting this to match Modal will aid migration and will address active feedback from teams.

Calcite package

Esri team

Calcite (design)

driskull commented 5 days ago

Modal was deprecated in favor of Dialog, but their background colors have different defaults. Correcting this to match Modal will aid migration and will address active feedback from teams.

Its just using the calcite-panel. Why would the panel use a different background color in this instance?

Modal was deprecated in favor of Dialog, but their background colors have different defaults. Correcting this to match Modal will aid migration and will address active feedback from teams.

@ashetland why not change panel or introduce a property on panel to configure the background appearance if it is different depending on certain use cases.

macandcheese commented 5 days ago

Prior to the change Modal behaved the same way (default to white, option to change via css property). Similarly to how we have a different default padding in Dialog than we do in Panel.

This should ease migration from Modal while also allowing Dialog to remain flexible for the increased set of use cases it serves - where both the default (now white) or other values (grey, primarily, but depending on use case other options) can be set via the existing css property.

ashetland commented 5 days ago

Changing the default background color of Panel would also require some design tweaks to Block and other components. It would be a large change in the design pattern.

driskull commented 5 days ago

I guess my concern is the inconsistency.

If we have valid use cases for when a panel should have a white background then maybe we should document them by having a prop where users can configure it rather than having to set CSS vars to a specific value.

macandcheese commented 5 days ago

This would be a direct response to users asking for this change - honestly I don't have concerns about that level of difference and I don't think we want to limit that level of customization. This is how Modal functioned before, all we are doing is making the new component match that prior default so migrating between components is simpler.

Making users adjust this via a property seems like an anti-pattern vs. using a css property.

Panel as a standalone component currently does not have many use cases where it shouldn't be the default grey (again, because the primary use case there is slotting in "foreground Blocks") on top of that. Dialog has different use cases, some of which may necessitate an override, but ultimately using the less opinionated white bg makes that a simpler story.

That relationship between Panel / Block will definitely be looked at and we have ideas to improve it, but that's more than a year away post 4.0 at this point.