Clean up Dark Mode borders around tables in Key Bindings & Advanced tabs #4129
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
c0d5bcd to
8f80d03
Compare
|
Eh. Revisited this and decided to come to terms with deleting some endearing but unnecessary code. Revised the description appropriately and force-pushed a simpler commit. |
8f80d03 to
4cdaeaa
Compare
|
Resubmitted - previous push had some lingering XIB junk which was generating warnings |
|
Sorry for ignoring this for so long. I think the changes are good based on the image you posted. Could you redo this PR on the latest develop branch? |
|
Redid the PR. Made a couple other minor fixes which popped out at me. See the commit msg for details. |
Changes: • Change border color of NSBoxes below Key Bindings, Configuration, & MPV Options tables from "Grid" to "Separator Color", so that their borders are consistent with the tables above even in Dark Mode • Adjust vertical offset of "Display raw values" checkbox to center it & add constraint to keep it aligned with "Duplicate…" button • Remove focus ring from MPV Options table, so it matches other tables (in the future this table should be migrated from cell-based to view-based to best ensure consistency)
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I came up with this while working on #4107, but that's already too big, and wanted to get comments on this, so I pulled this out and put it into its own issue.
Description:
[Updated from the original post: now much simpler - only needs border changes to 3
NSBoxes- but the changes here look a lot bigger because they are using the code from #4107 as a base...]I use Dark Mode for the most part, and it bothers me that the box colors don't match for the 3 tables in
Prefs > Key BindingsandPrefs > Advanced. They seem ok in Light Mode. But in Dark Mode the boxes on the bottom (with the +/- buttons) are a dark gray, while the table above uses a light gray border, and although they aren't misaligned, the color difference makes them look like they are.I first tried changing the borders of the scroll view to a darker color, so it matched the boxes below, but that looked a bit off. It looks like in most other parts of MacOS, the trend is to use light borders. XCode is a notable exception in that it seems to favor dark borders, but personally I don't find XCode very attractive or modern looking. Open to discussion though.
So I tried to find colors in the existing palette which worked for both light and dark, and
Separator Colorwas the winner - in fact it looks like that is what modernNSScrollViews are using. So this PR just changes the border color for the box views below them to match.Results:
Wanted to highlight this and see if anyone thinks this is a controversial change.