feat(overlay): replace OverlayContainer themeClass w/ addClass/removeClass methods #6975
Conversation
googlebot
added
the
cla: yes
jelbourn
force-pushed
the
overlay-container-class
branch
from
3c23fc5 to
f473a2d
Compare
src/cdk/overlay/overlay-container.ts
Outdated
| if (value) { | ||
| this._containerElement.classList.add(value); | ||
| } | ||
| addClass(cssClass: string) { |
There was a problem hiding this comment.
Since the getContainerElement() is public, I'm not sure that these methods are even necessary.
There was a problem hiding this comment.
Yeah, I considered that. I was worried that the change would be "too complicated" for people, but maybe I'm overthinking it. I'll just remove the themeClass completely.
There was a problem hiding this comment.
IMO we still need some easy way for people to pass the theme to overlays, though. Perhaps a provider that injects the overlay container and applies the class?
There was a problem hiding this comment.
Is getContainerElement().classList.add('dark-theme') that must worse than themeClasss = 'dark-theme';?
There was a problem hiding this comment.
I don't think it is, but I guess that it's not as obvious that that's the way to theme all components.
There was a problem hiding this comment.
IMO while getContainerElement().classList.add('dark-theme') isn't immediately obvious, it reduces the magic, which is nice.
Just don't forget the example in the theming guide needs to be updated with whichever solution!
jelbourn
added
the
P1
jelbourn
force-pushed
the
overlay-container-class
branch
2 times, most recently
from
5ad9bf9 to
593aa7b
Compare
|
I went ahead and just removed |
guides/theming.md
Outdated
| (`.unicorn-dark-theme` in the example above). | ||
|
|
||
| To do this, you can specify a `themeClass` on the global overlay container. For the example above, | ||
| To do this, you can add the approviate class to the global overlay container. For the example above, |
crisbeto
left a comment
crisbeto
left a comment
There was a problem hiding this comment.
LGTM after correcting the typo.
jelbourn
force-pushed
the
overlay-container-class
branch
from
593aa7b to
7dca378
Compare
…Class methods
BREAKING CHANGE: Now that the Overlay is part of the cdk rather than Angular Material directly,
the `themeClass` property has been removed. To add a class to the
overlay for theming, you can do
```ts
overlayContainer.getContainerElement().classList.add('my-theme-class');
```
jelbourn
force-pushed
the
overlay-container-class
branch
from
7dca378 to
fafe4c5
Compare
|
Fixed typo and demo-app (which I missed before) |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
4 participants
BREAKING CHANGE: Now that the Overlay is part of the cdk rather than Angular Material directly,
the
themeClassproperty has been removed. To add a class to theoverlay for theming, you can do