Skip to content

Conversation

@AntonioVentilii
Copy link
Contributor

@AntonioVentilii AntonioVentilii commented Jul 7, 2025

Motivation

Just an idea: it would be easier to customize the Toast style using CSS styles, if we add the level among the classes.

For example, in OISY, if we want to customize the background of an error toast we need to use the icon child:

.toast:has(> .icon.error) {
	--overlay-background: var(--color-background-error-light);
	--overlay-background-contrast: var(--color-foreground-error-secondary);
}

While we could do:

.toast {
	--overlay-background-error: var(--color-background-error-light);
	--overlay-background-contrast-error: var(--color-foreground-error-secondary);
}

So, we define custom CSS classes for each level of Toast: background and foreground, inverted or not.

@AntonioVentilii AntonioVentilii marked this pull request as ready for review July 7, 2025 13:57
@AntonioVentilii AntonioVentilii requested review from a team as code owners July 7, 2025 13:57
@peterpeterparker
Copy link
Member

Or CSS variables?

Something like

.toast{
	--overlay-background-error: var(--color-background-error-light);
}

Not sure what's the common pattern but, I guess we rarely assume the consumer uses CSS selector, we probably do so in OISY as a hack...

@AntonioVentilii
Copy link
Contributor Author

AntonioVentilii commented Jul 9, 2025

@peterpeterparker you mean creating an --overlay-background-* for each level (or maybe just for error in our case), right? That would be coherent with how we normally use GIX-cmp

Basically:

<!-- Toast -->

...

<div
  data-tid="toast-component"
  role="dialog"
  class={`toast ${theme ?? "themed"}`}
  style={`--overlay-background: var(--toast-background-${level}, var(--overlay-background));
          --overlay-background-contrast: var(--toast-background-contrast-${level}, var(--overlay-background-contrast));
          --toast-inverted-background: var(--toast-inverted-background-${level}, var(--toast-inverted-background));
          --toast-inverted-background-contrast: var(--toast-inverted-background-contrast-${level}, var(--toast-inverted-background-contrast));`}
  in:fly|global={{ y: (position === "top" ? -1 : 1) * 100, duration: 200 }}
  out:fade|global={{ delay: 100 }}
>
....

WDYT?

@peterpeterparker
Copy link
Member

I was thinking defining those in the <style /> tag based on the class selector but, yes.

@AntonioVentilii
Copy link
Contributor Author

AntonioVentilii commented Jul 9, 2025

@peterpeterparker ah understood! Look at last commit pls, is that?

Copy link
Member

@peterpeterparker peterpeterparker left a comment

Choose a reason for hiding this comment

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

Wait so it was already there, we just want to have more flexibility, is that right?

If so, can you update title and description of the PR accordingly?

@AntonioVentilii AntonioVentilii changed the title feat: Define level-class for Toast component feat: Define CSS classes for levels of Toast component Jul 9, 2025
@AntonioVentilii
Copy link
Contributor Author

@peterpeterparker ready for another review pls

Copy link
Member

@peterpeterparker peterpeterparker left a comment

Choose a reason for hiding this comment

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

LGTM, thx

@AntonioVentilii AntonioVentilii merged commit af1b321 into main Jul 9, 2025
13 checks passed
@AntonioVentilii AntonioVentilii deleted the feat/Define-level-class-for-Toast-component branch July 9, 2025 10:31
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