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

Styling issues with native <dialog> element #2409

Closed
LoremFooBar opened this issue Jan 12, 2024 · 2 comments
Closed

Styling issues with native <dialog> element #2409

LoremFooBar opened this issue Jan 12, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@LoremFooBar
Copy link

LoremFooBar commented Jan 12, 2024

Current Behavior

There are some styling issues with native dialog elements:

  1. Some theme styles are not applied on dialogs. The most obvious one is font color.
  2. Using token classes for backdrops works on Firefox, but not on Chrome.

Expected Behavior

  • It should be possible to style backdrops with tokens on Chrome.
  • Dialogs should inherit theme styles like any other element.

Steps To Reproduce

From the repro:

<script lang="ts">
  let dialogElement: HTMLDialogElement;
  let dialogElement2: HTMLDialogElement;
</script>

<button class="btn variant-ghost-primary" on:click={()=>dialogElement.showModal()}>
  Open dialog with token backdrop
</button>
<button class="btn variant-ghost-primary" on:click={()=>dialogElement2.showModal()}>
  Open dialog with regular bg color backdrop
</button>


<dialog bind:this={dialogElement} class="backdrop:bg-surface-backdrop-token">
  Dialog content
</dialog>

<dialog bind:this={dialogElement2} class="backdrop:bg-black/50">
  Dialog content
</dialog>

Link to Reproduction / Stackblitz

https://github.com/LoremFooBar/skeleton-native-dialog-styling-issue

More Information

I added screenshots from Chrome and Firefox to the readme in the repro repo demonstrating the issues.

@LoremFooBar LoremFooBar added the bug Something isn't working label Jan 12, 2024
@endigo9740
Copy link
Contributor

endigo9740 commented Jan 13, 2024

@LoremFooBar, so a couple things here:

  1. Support for the native Dialog element is currently outside the scope of Skeleton. I'd love to include it, but it feels a bit like a half-baked implementation in most browsers and doesn't sync well with the reactive patterns of Svelte. As such, we've opted for a custom modal system.
  2. You may have saw our recent post regarding progress towards Skeleton v3. We're reviewing many of the foundation features of Skeleton as part of this initiative. This includes a potential switch from :root -> :host scope when declaring CSS custom properties (aka CSS variables) used for theme settings. See that noted here for review.

I don't have a complete understanding of the native Dialog element under the hood, but if it utilizes a shadow DOM, then a move to :host in v3 will resolve some of these issues. It'll be one of many things we're testing in this regard. But we have to verify it doesn't introduce other gotchas or regressions. I would encourage you to keep an eye on progress going forward.

Given I don't think there's anything actionable for us in this ticket, I will close this out. However, I will go ahead and reference this in our v3 modal updates. I cannot promise we'll move forward with the native Dialog, but it honestly can't hurt to give it another look to confirm our stance.

Hope that helps!

@LoremFooBar
Copy link
Author

Thanks for the detailed reply!

The reason I tried to use a native <dialog> in the first place was because I tried to do something that I couldn't do with Skeleton's Modal. But after fighting with the <dialog> element for too long, it eventually dawned on me that I actually didn't need a modal at all...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants