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

Confusing API: WPFUI.Background.Manager vs. WPFUI.Theme.Manager #48

Closed
Aelarion opened this issue Jan 29, 2022 · 0 comments
Closed

Confusing API: WPFUI.Background.Manager vs. WPFUI.Theme.Manager #48

Aelarion opened this issue Jan 29, 2022 · 0 comments
Labels

Comments

@Aelarion
Copy link
Contributor

Aelarion commented Jan 29, 2022

I realize this may be a bit nitpicky and super broad, so feel free to close if this is considered non-actionable.

Context:
When trying to utilize the functionality of each of the Manager classes in WPFUI.Background and WPFUI.Theme I am finding myself getting confused quite often. Using intellisense as a crutch may be part of the problem, so no issue taking flak for that. However the current structure limits the ability to be clear when utilizing both types of managers, and also introduces a bit of ambiguity between the responsibilities of each manager.

Why it's confusing:
While I do understand the logical structure of these (e.g. Background.Manager refers more to the Win32 API and various window handle things, Theme.Manager strictly messes with styles and colors, but does occasionally touch some interop stuff), if you happen to need to touch each of these classes in a single class you really have to read the implementations to understand what you're looking for. I don't think Background captures the separation from Theme well enough given how much heavy lifting each of these managers are doing. Also, you are left with the option of basically not utilizing using statements for both WPFUI.Background and WPFUI.Theme, since you will get ambiguity errors and have to deconflict them anyway. This results in having to directly call which manager you need, which you may not specifically have memorized for what you're trying to do.

Inconsistent Public Methods:
There are further some publicly accessible method naming inconsistencies which don't help clarify things. For example, Theme.Manager uses a Switch verb to indicate we're changing a color scheme, but Background.Manager uses an Apply verb to indicate we're changing background effects. Both these effectively do the same thing -- they "change" value(s) from what is currently active to what we want to be active (with appropriate cleanup under the covers). To throw in some extra convolutions, we actually have Background.Manager.ApplyDarkMode and Background.Manager.RemoveDarkMode which are indirectly called by Theme.Manager.Switch. Part of the solution might be internalizing some stuff that isn't necessarily intended to be touched by the outside implementations -- I'm not familiar enough with these to know if that's feasible.

Possible fix:
I realize due to the complexity of these managers, this isn't something that could ever be done with a simple copy/paste or Ctrl+F replacements. However, I think perhaps instead of having "Background" and "Theme" being separate namespaces/directories, these could be combined under one larger namespace like WPFUI.WindowVisuals -- or something more generic. We could then have a BackgroundManager class and ThemeManager class in this namespace which clearly handle each of their respective duties. I realize this would require additional refactoring to avoid a monolithic and crowded namespace (e.g. for SystemManager and such) so we would obviously need to look at really planning out what makes sense.

Of course, I think whatever is chosen (if chosen to touch this) would likely have to be broken up into smaller, more manageable issues (e.g. "create ThemeManager class", "refactor ____ class", "remove ____ namespace", etc.).

@pomianowski pomianowski added the enhancement New feature or request label Jan 30, 2022
@pomianowski pomianowski pinned this issue Feb 13, 2022
@pomianowski pomianowski unpinned this issue May 30, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
Status: Done
Development

No branches or pull requests

2 participants