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

Auto theme #123

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Auto theme #123

wants to merge 15 commits into from

Conversation

btoms20
Copy link
Contributor

@btoms20 btoms20 commented Jan 21, 2025

What:

  • A couple convenience initializers for theme-ing your ChatView

Simply changing the accent color of the ChatView

  • This method keeps all of the default colors and changes only the ExyteBlue used throughout the ChatView.
// Creates and applies a `ChatTheme` to your `ChatView` based around the provided `accentColor`
public func chatTheme(accentColor: Color) -> some View

Coral-Light-Accent-Theme

Comprehensive Theme

  • This method mixes the theme color into the default background colors and, by default, adjusts the message bubbles for increase contrast (for better readability) when the provided color is either very light or very dark.
/// Creates and applies a `ChatTheme` to your `ChatView` based around the provided `color`
@available(iOS 18.0, *)
public func chatTheme(color ac: Color, background: ThemedBackgroundStyle = .mixedWithAccentColor(), improveContrast: Bool = true) -> some View

Cyan-Dark-Theme

Requires:

#122 to be merged in order for the theme to be applied correctly

Disclosure:

  • Feel free to modify this PR or delete it if deemed out of scope.
  • I modified the ChatExample App to showcase the new theme functions. This change / commit can be dropped before merging if you'd like to keep the original example app.

Important

The comprehensive theme requires iOS 18+ due to its use of the Color.mix(with:, by:) method.
Backwards compatibility is maintained by decorating these functions with @available(iOS 18.0, *)

…lying an accentColor to the ChatView. It handles switching certain images out for SFSymbols that are then tinted with said accent color.
…or auto-themeing a `ChatView`. By default it creates a ChatTheme with an accentColor and backgroundColor and adjusts some images to use SFSymbols instead of the bundled ones (allows for tinting of the icons). This method attempts to maintain contrast and readability when provided with very bright / dark colors by calcualting the luminance and adjusting it accordingly. This method is only available in iOS 18+ due to the use of `Color.mix(with:, by:)` method.
Example/ChatExample/ContentView.swift Outdated Show resolved Hide resolved
Example/ChatExample/ContentView.swift Outdated Show resolved Hide resolved
fullscreenTint: .white
)
)
// .mediaPickerTheme(
Copy link
Collaborator

Choose a reason for hiding this comment

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

what about prior to ios 18? will the media picker be not themed at all?

Copy link
Contributor Author

@btoms20 btoms20 Jan 22, 2025

Choose a reason for hiding this comment

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

Hey, the mediaPickerTheme was overridden in commit 88d097e.

This section in AttachmentsEditor.swift

.mediaPickerTheme(
    main: .init(
        text: theme.colors.mainText,
        albumSelectionBackground: theme.colors.mainBG,
        fullscreenPhotoBackground: theme.colors.mainBG,
        cameraBackground: theme.colors.mainBG,
        cameraSelectionBackground: theme.colors.mainBG),
    selection: .init(selectedTint: theme.colors.sendButtonBackground)
)

Should probably be replaced with just

.mediaPickerTheme(pickerTheme)

In order to honor the users ChatView.mediaPickerTheme modifier.

The problem is that the default values want to match that of the ChatTheme but if we apply these default values in the chatTheme modifier then we could potentially overwrite an already defined mediaPickerTheme.

ChatView( ... )
.mediaPickerTheme( ... custom theme ... )
.chatTheme( ... custom theme ... ) // If we set mediaPickerTheme defaults here, we override the changes made above.

If we bring mediaPickerTheme into the ChatTheme struct, then we could provide some nice default values without having to worry about potentially overriding a previously defined MediaPickerTheme.

ChatView( ... )
.chatTheme(
    colors: ...,
    images: ...,
    mediaPickerTheme: ...
)

What do you think?

Example/ChatExample/Screens/CommentsExampleView.swift Outdated Show resolved Hide resolved
Sources/ExyteChat/ChatView/MessageView/MessageMenu.swift Outdated Show resolved Hide resolved
Sources/ExyteChat/ChatView/MessageView/MessageMenu.swift Outdated Show resolved Hide resolved
Sources/ExyteChat/ChatView/Recording/RecordWaveform.swift Outdated Show resolved Hide resolved
colors: .init(
mainTint: accentColor,
messageMyBG: accentColor,
messageMyTimeText: Color.white.opacity(0.5),
Copy link
Collaborator

@f3dm76 f3dm76 Jan 22, 2025

Choose a reason for hiding this comment

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

why does white come together with accentColor? could you use your contrast magic for this too?

also, would you please move all of these to a separate file like "Theme+Convenience" or smth?

Copy link
Contributor Author

@btoms20 btoms20 Jan 22, 2025

Choose a reason for hiding this comment

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

The senders text is always .white, similar to your default ChatTheme value and Apple's message app. I was adjusting the message bubble color to improve contrast as opposed to altering the text color. I think a solid .white text color will always be easier to read over a tinted / off-white text on a colored background. What do you think?

@btoms20 btoms20 mentioned this pull request Jan 23, 2025
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