-
-
Notifications
You must be signed in to change notification settings - Fork 117
Pr/48 dark mode #69
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
Pr/48 dark mode #69
Conversation
Fixed an error where the asset path was not pointing to the right directory after forking the repo.
Implemented the possibility to easily choose the box style. Made it so we can potentially change the box style later, and got rid of the borders to avoid calculating textures on the fly.
@Ownezx super duper thank you for this. You caught me in the middle of a move across the country. I'll definitely take a detailed look at this. Super duper sorry for the delay in getting you a proper pull request review to move this through. Writing down a reminder in my tasks to get to this as soon as I'm settled in. Promise I will get to this, much excite. |
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.
This looks great. Some minor changes I can knock out if you don't have time. I still need to do a full manual regression check on the GUI display, but that's the only outstanding thing here.
PS Sorry this took so long for a review. I've been in moving limbo the past 6 months. Had a very "Murphy's Law" kinda move
|
||
public ColorFader MainIconFader { get; } | ||
|
||
public NodeFaders() { |
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.
Digging this, makes sense.
_box.Height - _box.PaddingY); | ||
|
||
if (_node.Task.HasBeenActive) { | ||
GUI.backgroundColor = _faders.BackgroundFader.CurrentColor; | ||
GUI.Box(rect, GUIContent.none, Styles.BoxActive.Style); | ||
|
||
if (EditorGUIUtility.isProSkin) { |
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.
Move if block to a static DisplaySettings.GetBoxActiveStyle()
method that can support caching
} | ||
|
||
else { | ||
if (EditorGUIUtility.isProSkin) { |
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.
Move if block to a static DisplaySettings.GetBoxInactiveStyle()
method that can support caching
I have added that on my todo-List. I haven't touched Unity in a few months though (It's my turn to be busy), so if you want the changes to implemented timely, don't wait on me. Take care. |
No worries, think I can tackle the minor touch-ups no problem. I totally get how schedules can fluctuate |
Sorry this is still in limbo. Been overloaded with game project(s) I'm on 😅 |
There seems to be an issue with docked and undocked editor GUIs, looking into it. Looks like a bug on Unity's end potentially :( |
Closing in favor of #97 |
Addressing issue #48.
First commit is just after pulling, I had errors when I pulled up the tree renderer as it could not find the images in the project.
Second is adressing dark mode.
All unit test still run as I have only done cosmetics.
I also removed the texture rendering to avoid recreating a texture for each boxes every frame. This has the disadvantage of loosing the box borders.