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

[BUGFIX] Minor UI tweaks #128

Merged
merged 10 commits into from
Jan 23, 2024
Merged

[BUGFIX] Minor UI tweaks #128

merged 10 commits into from
Jan 23, 2024

Conversation

DominikBucher12
Copy link
Collaborator

@DominikBucher12 DominikBucher12 commented Jan 18, 2024

What this PR do:

  • Fixes colors for buttons and pickers for dark and light mode
  • Fixes number of Color pickers

How to test it

  • Run app, check if it fits the screenshots

Related: #125

How it looks now:

macOS iOS
PR 1 PR 4
PR 2 PR 3

@DominikBucher12
Copy link
Collaborator Author

Now this doesnt work on macOS, investigating why... Will fix this later, got a bigger fish to catch.

@danielsaidi
Copy link
Owner

It's so strange that the toggle buttons work great in dark mode on macOS, but not in iOS.

I'll check if I can get it to work with the current approach before we merge this.

@DominikBucher12
Copy link
Collaborator Author

Even alignment buttons fixed in light mode!

Alignment fixed lightmode

@danielsaidi
Copy link
Owner

Thank you for finding all these strange glitches.

My idea was for a developer to be able to set foregroundStyle to an entire panel or sheet, and have all the icons adapt, but I guess we can wait with that :)

@danielsaidi
Copy link
Owner

Is this ready to merge?

@DominikBucher12
Copy link
Collaborator Author

I will refactor this a bit (no duplicate static systemSpecificTextColor, revert back the alignment light and dark initialiser...) and we are ready to roll... :)

@DominikBucher12
Copy link
Collaborator Author

I spent enormous time trying to hack SwiftUI.Picker to work with SFSymbols but nothing, literally NOTHING, except making custom Segment Picker works... So here is my 2 cents... Lets keep alignment stack this way, otherwise everything works.

@danielsaidi
Copy link
Owner

danielsaidi commented Jan 23, 2024

Hey @DominikBucher12 - I looked at the many things you had to do to make this work and realized that many of the views were sooo bloated, with view modifiers and styles that made it hard to affect the entire look of, say, a format sidebar.

I therefore removed a LOT of code and styles from the various views, so that you can now use foreground color, tint, accent color etc. on an entire view hierarchy and affect all toggles within. I also noticed that the palette symbol color mode was what caused many strange icon rendering issues, so I removed it.

When I did, I noticed that the ControlGroup ruins the highlight effect for nested toggles in iOS. I therefore hot switch out the toggle group with a toggle stack on iOS, to make it work as expected.

I've also removed the color picker color adjustment, since it made it impossible to e.c. pick a black background color in dark mode. Instead, I've added a reset button to the quick colors, which applies nil, which now translated to .primary to all color types, except for background, for which it applies .clear.

I have also deprecated a bunch of styles, and moved many views into namespaces. Let's go through this PR together and see which parts we still want to add to mail, and we can add them as smaller, separate PRs.

@DominikBucher12
Copy link
Collaborator Author

Looks like this tweaking SFSymbol/TintColor journey is finally over, ready to merge.

@danielsaidi danielsaidi merged commit 69580bb into main Jan 23, 2024
1 check passed
@danielsaidi danielsaidi deleted the bugfix/style_colors branch February 14, 2024 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants