Skip to content

Conversation

simeonoff
Copy link
Collaborator

@simeonoff simeonoff commented Dec 6, 2024

Closes #15094

Additional information (check all that apply):

  • Bug fix
  • New functionality
  • Documentation
  • Demos
  • CI/CD

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code (test guidelines)
  • This PR includes API docs for newly added methods/properties (api docs guidelines)
  • This PR includes feature/README.MD updates for the feature docs
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes ng update migrations for the breaking changes (migrations guidelines)
  • This PR includes behavioral changes and the feature specification has been updated with them

@simeonoff simeonoff changed the title Simeonoff/18.2.x fix 15094 fix(*): icon service doesn't work with scoped themes Dec 6, 2024
@dobromirts dobromirts added ✅ status: verified Applies to PRs that have passed manual verification and removed ❌ status: awaiting-test PRs awaiting manual verification labels Dec 6, 2024
@simeonoff simeonoff requested a review from damyanpetev December 9, 2024 13:56
damyanpetev
damyanpetev previously approved these changes Dec 9, 2024
Copy link
Member

@damyanpetev damyanpetev left a comment

Choose a reason for hiding this comment

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

LGTM

damyanpetev
damyanpetev previously approved these changes Dec 9, 2024
@damyanpetev damyanpetev self-requested a review December 9, 2024 16:15
public subject: BehaviorSubject<IgxTheme>;

constructor(private t?: IgxTheme) {
const globalTheme = globalThis.window
Copy link
Member

Choose a reason for hiding this comment

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

And I think I preferred this more on the root factory level or was this intentional for a sneaky auto-init on a scoped bit of the DOM? I guess that could be useful, just figured the main use would be to always set something anyway which would make the computed styles call redundant.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The sneaky thing this allows me to do is to initialize a ThemeToken without passing a theme to the constructor in the root factory, making the preferToken getter return false when no theme is passed to the factory.

@damyanpetev damyanpetev merged commit 217ea33 into 18.2.x Dec 10, 2024
5 checks passed
@damyanpetev damyanpetev deleted the simeonoff/18.2.x-fix-15094 branch December 10, 2024 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

In multi-theme applications, icons automatically include classes based on the global theme.

4 participants