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

fix: navigation bar colors #232

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Core/Core/Extensions/UIApplicationExtension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ extension UINavigationController {
navigationBar.shadowImage = UIImage()

let image = CoreAssets.arrowLeft.image
navigationBar.backIndicatorImage = image.withTintColor(Theme.Colors.accentColor.uiColor())
navigationBar.backIndicatorImage = image.withTintColor(Theme.UIColors.accentColor)
navigationBar.backItem?.backButtonTitle = " "
navigationBar.backIndicatorTransitionMaskImage = image.withTintColor(Theme.Colors.accentColor.uiColor())
navigationBar.titleTextAttributes = [.foregroundColor: Theme.Colors.textPrimary.uiColor()]
navigationBar.backIndicatorTransitionMaskImage = image.withTintColor(Theme.UIColors.accentColor)
navigationBar.titleTextAttributes = [.foregroundColor: Theme.UIColors.textPrimary]
}
}

Expand Down
13 changes: 11 additions & 2 deletions Core/Core/Extensions/ViewExtension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
//

import Foundation
import SwiftUIIntrospect
@_spi(Advanced) import SwiftUIIntrospect
import SwiftUI
import Theme

Expand Down Expand Up @@ -193,13 +193,22 @@ public extension View {
} else {
return self.introspect(
.navigationView(style: .stack),
on: .iOS(.v14, .v15, .v16, .v17),
on: .iOS(.v15...),
scope: .ancestor) {
$0.isNavigationBarHidden = true
}
}
}

func navigationBackground(color: UIColor) -> some View {
return self.introspect(
.navigationView(style: .stack),
on: .iOS(.v15...),
scope: .ancestor) {
$0.navigationBar.barTintColor = color
}
}

func hideScrollContentBackground() -> some View {
if #available(iOS 16.0, *) {
return self.scrollContentBackground(.hidden)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ extension ScrollSlidingTabBar {
public static let `default` = Style(
font: .body,
selectedFont: .body.bold(),
activeAccentColor: Theme.Colors.accentColor ,
inactiveAccentColor: .black.opacity(0.4),
activeAccentColor: Theme.Colors.accentColor,
inactiveAccentColor: Theme.Colors.textSecondary,
indicatorHeight: 2,
borderColor: .gray.opacity(0.2),
borderHeight: 1,
Expand Down
4 changes: 2 additions & 2 deletions Core/Core/View/Base/WebUnitView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
//

import SwiftUI
import SwiftUIIntrospect
@_spi(Advanced) import SwiftUIIntrospect
import Theme

public struct WebUnitView: View {
Expand Down Expand Up @@ -62,7 +62,7 @@ public struct WebUnitView: View {
.frame(width: reader.size.width, height: reader.size.height)
}
}
.introspect(.scrollView, on: .iOS(.v14, .v15, .v16, .v17), customize: { scrollView in
.introspect(.scrollView, on: .iOS(.v15...), customize: { scrollView in
scrollView.isScrollEnabled = false
})
if viewModel.updatingCookies || isWebViewLoading {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ public struct CourseContainerView: View {
.navigationBarBackButtonHidden(false)
.navigationTitle(titleBar())
.onChange(of: selection, perform: didSelect)
.navigationBackground(color: Theme.UIColors.background)
.background(Theme.Colors.background)
}

@ViewBuilder
Expand Down
16 changes: 16 additions & 0 deletions Theme/Theme/Theme.swift
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,22 @@ public struct Theme {
}
}

public struct UIColors {
public private(set) static var textPrimary = ThemeAssets.textPrimary.color
public private(set) static var accentColor = ThemeAssets.accentColor.color
public private(set) static var background = ThemeAssets.background.color

public static func update(
textPrimary: UIColor = ThemeAssets.textPrimary.color,
accentColor: UIColor = ThemeAssets.accentColor.color,
background: UIColor = ThemeAssets.background.color
) {
self.textPrimary = textPrimary
self.accentColor = accentColor
self.background = background
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rnr Why it's needed to create a new struct UIColors when we already have uiColor() extension written on Colors?

Even if we want to define categorical UIColors, I guess it would be nice to have all the values that Colors have and also it would be nice if you can remove uiColor() extension so we will have a single source. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@saeedbashir
I'm not sure we need to add the whole bunch of colors to the UIColors structure - it seems like unnecessary duplication.
I experimented again and found that only 'accentColor' and 'textPrimary' in this structure is sufficient. Calculated UIColors get broken when we use these in UIApplicationExtension.swift for override func viewWillLayoutSubviews(). For all other places (like CCSInjection or Discussions) it looks working.
So uiColor() is helpful extension and I don't think we need to delete this.
I commit small changes (got rid unnecessary navigation background modifier) . Please review.

public struct Fonts {

public static let displayLarge: Font = .custom(fontsParser.fontName(for: .regular), size: 57)
Expand Down