bsupnik / bricksmith

LDraw Modeler for OS X
BSD 2-Clause "Simplified" License
12 stars 6 forks source link

Dark mode fixes and minor project updates #12

Closed undecoded-coder closed 2 years ago

undecoded-coder commented 2 years ago

Dark mode fixes, asset updates and migration to asset catalog, and other minor changes.

At a glance:

[Updated PR description]

undecoded-coder commented 2 years ago

@allenmonroesmith if there's anything I could do to help this project along let me know! Thanks!

allenmonroesmith commented 2 years ago

Please make PRs for any other fixes and improvements you have made.

From a technical perspective, the most critical issues are:

undecoded-coder commented 2 years ago

Please let me know if the application is releasable with dark mode right now.

I'll look through some of the auxiliary windows just to make sure, but so far (with these changes) it's been fine. The only thing I have yet to solve is a slight change between window and row background color in the file outline (Edit: this was due to the text field cell drawing its own background via the checkbox in Interface Builder; on a separate note, I may try converting this to be view-based instead of cell based at some point).

undecoded-coder commented 2 years ago

Minor text color fix! Beyond that it all looks great now.

The only black-on-dark thing I can find is the icon for line primitives in the file outline. In order to fix that though I think the best thing would be to migrate to using Asset Catalogs (which is orthogonal to this PR).

@allenmonroesmith, I think you'll have to review changes again in order to merge the PR (if everything looks good to you), but I don't know of any other changes to make on this right now, so you can merge it at will.

Edit: I do need to look into the row background color (in Dark Mode) in the outline still.

undecoded-coder commented 2 years ago

That was easy.

undecoded-coder commented 2 years ago

I'll post any other Dark Mode issues on #7 as I find them

undecoded-coder commented 2 years ago

I was hoping to put the asset catalog migration changes into a separate pull request but it didn't turn out that way.

Note: I did not delete the original assets folder yet (though I removed it from the application target) even though it is no longer needed. Let me know if I should, or feel free to do so yourself.

undecoded-coder commented 2 years ago

Okay, I tried adding some new placard icons since the current ones have bright backgrounds.

I could try to just replicate the existing ones without the backgrounds, but I thought I could try these on for size first. That said, it may be better to use alternate images for Dark Mode (now that they are in an asset catalog where you can do that). Let me know what you think!

For comparison, here are some screenshots: Screen Shot 2022-01-12 at 4 14 44 PM Screen Shot 2022-01-12 at 4 14 18 PM Screen Shot 2022-01-12 at 4 16 07 PM Screen Shot 2022-01-12 at 4 13 36 PM

undecoded-coder commented 2 years ago

It says only those with write access to this repository can merge pull requests. So that would be you. :]