-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Ability to have theme overrides for a widget/container #4313
Conversation
…n of new methods. Will probably deprecate the helpers - not decided yet...
Starting with Color in Label and Button
Also we must reset the cache when override is called as we may have cached the wrong colours
…atching `Current` for fallback
This isn't the whole codebase but it's the APIs and techniques required. Rather than drag on a large PR for much longer and get them all I'd like to land this one as soon as the API/code structure etc is approved and track remaining widgets in another issue with multiple PRs. |
Edit, the ID update is easier - using the scope of different caches to simply prefix the IDs works quite well. Just working on how the import loop issue for ThemedResource may work for the |
You might be able to change |
The colour name type is already top level, it's being able to look up the new value that's a challenge. I've realised there are two solutions:
I think 1 wins - 2 feels a little hacky for a reason I can't quite put my finger on. |
Also keep more info about widget/resource pairing internal to cache. And break the theme/cache interdependence with a top level abstraction of ThemedResource
This reverts commit 48fbb19.
Is this looking good now @dweymouth, @Jacalz ? |
Thanks @Jacalz - very exciting :) I think that @dweymouth has a bit of an outage but I'll wait a little longer in case they are back and have time to confirm the fix. |
Yep, still have intermittent internet access. Hopefully should be fully back online by end of tomorrow, but no guarantees. I will try to take a look at this later in the coming week |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with the requirement that an issue is opened as a blocker against v2.5.0 to make sure that the last remaining improvements are resolved before then.
As discussed at contributor meeting, API updated to simpler and less exposed info.
Does not include all widgets, that will be a follow-on PR.
Additional optimisations could follow to reduce time spent in setting/walking the list of overridden widget themes.
Checklist:
Where applicable: