Convert internal, static colors into color assets. #1386
Merged
Conversation
This file contains 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
Matches audio tint more closely to iMessage by tinting white on outgoing and tinting blue on incoming
Matches dot color more closely to iMessage
Also removes redundant internal access level modifier on MessageKitError (internal is default), and changes MessageKitError from enum -> struct as its members were not enum cases
|
Screenshots showing fixes to the video play button: |
Codecov Report
@@ Coverage Diff @@
## development #1386 +/- ##
===============================================
+ Coverage 64.75% 64.99% +0.24%
===============================================
Files 68 68
Lines 3305 3277 -28
===============================================
- Hits 2140 2130 -10
+ Misses 1165 1147 -18
Continue to review full report at Codecov.
|
Merged
Patrick-Kladek
pushed a commit
to medbee/MessageKit
that referenced
this pull request
Oct 25, 2023
* Add Assets.xcassets to MessageKitAssets.bundle * Replace incomingGray, outgoingGreen, inputBarGray * Replace playButtonLightGray, with UIVisualEffectView to mimic iMessage (Fixes MessageKit#1321 , Fixes MessageKit#1335) * Fixes an issue where the triangle in the play button is not centered * remove sendButtonBlue, add incoming/outgoingAudioMessageTint Matches audio tint more closely to iMessage by tinting white on outgoing and tinting blue on incoming * Remove backgroundColor, add collectionViewBackground * remove unused labelColor * Remove unused placeholderTextColor * Remove grayColor * remove darkTextColor, add typingIndicatorDot Matches dot color more closely to iMessage * Remove unused lightGrayColor * Adds high contrast appearances to the color assets * Add MessageKitError string for case when color asset could not be loaded Also removes redundant internal access level modifier on MessageKitError (internal is default), and changes MessageKitError from enum -> struct as its members were not enum cases * Fix swiftlint issues * Add label color asset and use in ContactMessageCell * Fix tests
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.
What does this implement/fix? Explain your changes.
As described by @austinwright in issue Convert internal, static colors into color assets. #1301, this PR replaces the static
UIColorconstants with Xcode Color Assets, which are edited visually in Xcode, allowing you to combine appearances for dark/light mode, and high contrast in one described 'color. These color assets are named for the use of the color rather than the color value itself.Also fixes an issue where the video play button was too dark in dark mode (Issue Video message playback icon bug in dark mode #1335). Specifically, replaces the background color of the video playback button with a
UIVisualEffectsViewwhich blurs the background similar to how it looks on iMessage.Also fixes an issue where the video play triangle was not properly centered in the button view (no issue #).
Does this close any currently open issues?
Yes:
Closes #1301
Closes #1335
Closes #1321
Any other comments?
I imagine there is a small performance impact of using a
UIVisualEffectsViewin the video play button as opposed to a plain color background however I quickly tested by profiling scrolling through 99 video messages on an iPhone 6 (a 6 year-old phone!) in the example app and I was able to scroll up to 60FPS.The addition of the
UIVisualEffectsViewcould also be its own PR, but I added it since I was changing the background color there anyways and its a small change.For now, I added static vars in the extension of the
UIColorclass to make the references to the color assets type-safe, but this is something we could maybe have automated for us like with SwiftGen. That could be its own PR in the future.Where has this been tested?
Devices/Simulators: … iPhone 11 Simulator, iPhone 6 Device
iOS Version: … iOS 12, and iOS 13
Swift Version: … Swift 5
MessageKit Version: … 3.1.0