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

[Dialog] - content background should be white #10809

Open
2 of 6 tasks
ashetland opened this issue Nov 20, 2024 · 5 comments
Open
2 of 6 tasks

[Dialog] - content background should be white #10809

ashetland opened this issue Nov 20, 2024 · 5 comments
Assignees
Labels
2 - in development Issues that are actively being worked on. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. Calcite (design) Issues logged by Calcite designers. calcite-components Issues specific to the @esri/calcite-components package. estimate - 1 Very small fix or change (potentially a single line), doesn't require updates to tests. impact - p1 - need for current milestone User set priority impact status of p1 - need for current milestone p - high Issue should be addressed in the current milestone, impacts component or core functionality visual changes Issues with visual changes that are added for consistency, but are not backwards compatible

Comments

@ashetland
Copy link

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/calcite-components
  • @esri/calcite-components-react
  • @esri/calcite-design-tokens
  • @esri/calcite-ui-icons
  • @esri/eslint-plugin-calcite-components

Esri team

Calcite (design)

@ashetland ashetland added bug Bug reports for broken functionality. Issues should include a reproduction of the bug. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Nov 20, 2024
@github-actions github-actions bot added Calcite (design) Issues logged by Calcite designers. calcite-components Issues specific to the @esri/calcite-components package. impact - p1 - need for current milestone User set priority impact status of p1 - need for current milestone labels Nov 20, 2024
@ashetland ashetland changed the title [Dialog] - content background should white [Dialog] - content background should be white Nov 20, 2024
@geospatialem geospatialem added estimate - 1 Very small fix or change (potentially a single line), doesn't require updates to tests. visual changes Issues with visual changes that are added for consistency, but are not backwards compatible p - high Issue should be addressed in the current milestone, impacts component or core functionality and removed needs triage Planning workflow - pending design/dev review. labels Nov 20, 2024
@driskull
Copy link
Member

driskull commented Nov 20, 2024

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
Copy link
Contributor

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
Copy link
Author

ashetland commented Nov 20, 2024

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
Copy link
Member

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
Copy link
Contributor

macandcheese commented Nov 20, 2024

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.

@jcfranco jcfranco self-assigned this Nov 23, 2024
@jcfranco jcfranco added 2 - in development Issues that are actively being worked on. and removed 0 - new New issues that need assignment. labels Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 - in development Issues that are actively being worked on. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. Calcite (design) Issues logged by Calcite designers. calcite-components Issues specific to the @esri/calcite-components package. estimate - 1 Very small fix or change (potentially a single line), doesn't require updates to tests. impact - p1 - need for current milestone User set priority impact status of p1 - need for current milestone p - high Issue should be addressed in the current milestone, impacts component or core functionality visual changes Issues with visual changes that are added for consistency, but are not backwards compatible
Projects
None yet
Development

No branches or pull requests

5 participants