Update UI in location sharing view #5720 #5859

MaximeEvrard42 · 2022-03-17T16:00:34Z

iPhone 8 plus Light

iPhone 11 Pro Dark

github-actions · 2022-03-17T16:34:35Z

📱 Scan the QR code below to install the build for this PR.
🔒 This build is for internal testing purpose. Only devices listed in the ad-hoc provisioning profile can install Element Alpha.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/C6Cuwu

SBiOSoftWhare

Can you add a screenshot for light and dark theme please?

Riot/Modules/Room/Location/LocationMarkerView.swift

SBiOSoftWhare · 2022-03-18T09:55:59Z

Riot/Modules/Room/Location/LocationMarkerView.swift

@@ -20,14 +20,23 @@ import Mapbox

 class LocationMarkerView: MGLAnnotationView, NibLoadable {

+ @IBOutlet private var markerBackground: UIImageView!


Because we are in the LocationMarkerView maybe we can use this kind of name:

Suggested change

@IBOutlet private var markerBackground: UIImageView!

@IBOutlet private var backgroundImageView: UIImageView!

Riot/Modules/Room/Location/LocationMarkerView.swift

SBiOSoftWhare · 2022-03-18T10:08:22Z

Riot/Modules/Room/Location/LocationMarkerView.swift

 override func awakeFromNib() {
 super.awakeFromNib()
 translatesAutoresizingMaskIntoConstraints = false
 }

 func setAvatarData(_ avatarData: AvatarViewDataProtocol) {
+ Self.usernameColorGenerator.defaultColor = theme.colors.primaryContent
+ Self.usernameColorGenerator.userNameColors = theme.colors.namesAndAvatars


As we are starting to use the UserNameColorGenerator in several places we can probably factorize the theming part like this in the UserNameColorGenerator class:

// MARK: - Themable extension UserNameColorGenerator: Themable { func update(theme: Theme) { self.defaultColor = theme.colors.primaryContent self.userNameColors = theme.colors.namesAndAvatars } }

RiotSwiftUI/Modules/Room/LocationSharing/View/LocationSharingUserMarkerView.swift

SBiOSoftWhare · 2022-03-18T10:20:09Z

RiotSwiftUI/Modules/Room/LocationSharing/View/LocationSharingOptionButtonIcon.swift

+ .frame(width: 40, height: 40)
+ Image(uiImage: image)
+ .renderingMode(.template)
+ .foregroundColor(Color.white)


We should use Theme colors

SBiOSoftWhare · 2022-03-18T10:21:17Z

RiotSwiftUI/Modules/Room/LocationSharing/View/LocationSharingView.swift

+ LocationSharingOptionButton(text: VectorL10n.locationSharingLiveShareTitle) {
+ // TODO: - Start live location sharing
+ } content: {
+ LocationSharingOptionButtonIcon(fillColor: Color.purple, image: Asset.Images.locationLiveIcon.image)


We should use Theme colors. If it does not exist yet we should add a property. In this case we should add the color to ThemeSwiftUIType directly.

SBiOSoftWhare · 2022-03-18T10:24:12Z

RiotSwiftUI/Modules/Room/LocationSharing/View/LocationSharingOptionButton.swift

+ let action: () -> (Void)
+ @ViewBuilder var content: Content
+
+ var body: some View {


Does the parent accent color apply to the background if you change the theme?

SBiOSoftWhare · 2022-03-18T10:26:19Z

RiotSwiftUI/Modules/Room/LocationSharing/View/LocationSharingUserMarkerView.swift

 // MARK: Public

+ let isMarker: Bool


Can we add a small comment to indicate what does it mean

I think we can create a reusable BorderedAvatarImage that have a UserNameColorGenerator property in view model or view. And then integrate this BorderedAvatarImage in LocationSharingUserMarkerView as subview. LocationSharingUserMarkerView and BorderedAvatarImage shared the same UserNameColorGenerator instance to set the right color.

SBiOSoftWhare · 2022-03-23T15:34:56Z

RiotSwiftUI/Modules/Room/LocationSharing/View/LocationSharingOptionButton.swift

+
+ } buttonIcon: {
+ AvatarImage(avatarData: AvatarInput(mxContentUri: nil, matrixItemId: "Alice", displayName: "Alice"), size: nil)
+ .shapedBorder(color: Color.green, borderWidth: 3, shape: Circle())


Use convenient method quoted above: AvatarImage(avatarData: AvatarInput(mxContentUri: nil, matrixItemId: "Alice", displayName: "Alice")).border(color: theme.colors.accent)

SBiOSoftWhare · 2022-03-23T15:39:09Z

RiotSwiftUI/Modules/Room/LocationSharing/View/LocationSharingOptionButton.swift

+
+ } buttonIcon: {
+ AvatarImage(avatarData: AvatarInput(mxContentUri: nil, matrixItemId: "Alice", displayName: "Alice"), size: nil)
+ .shapedBorder(color: Color.green, borderWidth: 3, shape: Circle())


We should use colors from ThemeSwiftUI, in this case the color is theme.colors.accent

SBiOSoftWhare · 2022-03-23T15:41:21Z

RiotSwiftUI/Modules/Room/LocationSharing/View/LocationSharingView.swift

+ context.send(viewAction: .share)
+ } buttonIcon: {
+ AvatarImage(avatarData: context.viewState.userAvatarData, size: nil)
+ .shapedBorder(color: theme.displayUserColor(for: context.viewState.userAvatarData.matrixItemId), borderWidth: 3, shape: Circle())


Use convenient border() extension method quoted above.

SBiOSoftWhare · 2022-03-23T15:41:38Z

RiotSwiftUI/Modules/Room/LocationSharing/View/UserLocationAnnotatonView.swift

- guard let avatarImageView = UIHostingController(rootView: LocationSharingUserMarkerView(avatarData: avatarData)).view else {
+ guard let avatarImageView = UIHostingController(rootView: LocationSharingMarkerView(backgroundColor: theme.displayUserColor(for: avatarData.matrixItemId)) {
+ AvatarImage(avatarData: avatarData, size: nil)
+ .shapedBorder(color: theme.displayUserColor(for: avatarData.matrixItemId), borderWidth: 3, shape: Circle())


Use convenient border(color: Color) extension method quoted above.

SBiOSoftWhare · 2022-03-23T15:53:44Z

RiotSwiftUI/Modules/Common/Avatar/View/AvatarImage.swift

@@ -59,7 +66,7 @@ struct AvatarImage: View {



To help factorize BorderModifier usage:

Suggested change

@available(iOS 14.0, *)

extension AvatarImage {

func border(color: Color) -> some View {

modifier(BorderModifier(color: color, borderWidth: 3, shape: Circle()))

}

/// Use display name color as border color by default

func border() -> some View {

let borderColor = theme.displayNameColor(for: matrixItemId)

return self.border(color: borderColor)

}

}

RiotSwiftUI/Modules/Common/Util/BorderModifier.swift

DesignKit/Source/Colors.swift

SBiOSoftWhare · 2022-03-23T16:17:34Z

DesignKit/Source/Colors.swift

+ // MARK: - Others colors
+
+ /// White
+ var white: ColorType { get }


The property name should reflect the purpose. It seems that this color is not used.

We should also create another protocol to handle other colors. Colors.swift handles only colors from Figma Compound. We need to create public protocol OtherColors in a separated file and add otherColors property on ThemeV2 and ThemeSwiftUIType.

I removed here, they was no longuer necessary. For the protocol, I am looking how colors was handle in former project, I'll work on it later.

RiotSwiftUI/Modules/Room/LocationSharing/View/LocationSharingMarkerView.swift

Riot/Modules/Room/Location/RoomTimelineLocationView.swift

MaximeEvrard42 added 2 commits March 17, 2022 16:52

5720: Update UI in location sharing View

83a718b

5720: Changelog

9ca2b1d

MaximeEvrard42 requested a review from SBiOSoftWhare March 17, 2022 16:00

SBiOSoftWhare reviewed Mar 18, 2022

View reviewed changes

MaximeEvrard42 added 3 commits March 22, 2022 09:41

5720: Update from develop

49144e1

5720: Rework some location sharing related view

e805e41

5720: Fix issue with cornered buttons view

db8c5cc

SBiOSoftWhare reviewed Mar 23, 2022

View reviewed changes

5720: Remove useless color, fix some issue with AvatarImage border

064c974

SBiOSoftWhare approved these changes Mar 24, 2022

View reviewed changes

MaximeEvrard42 merged commit 5a4c5e5 into develop Mar 24, 2022

MaximeEvrard42 deleted the maximee/5720_location_sharing_UI_change branch March 24, 2022 10:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update UI in location sharing view #5720 #5859

Update UI in location sharing view #5720 #5859

MaximeEvrard42 commented Mar 17, 2022 •

edited

Loading

github-actions bot commented Mar 17, 2022 •

edited

Loading

SBiOSoftWhare left a comment

SBiOSoftWhare Mar 18, 2022

SBiOSoftWhare Mar 18, 2022

SBiOSoftWhare Mar 18, 2022

SBiOSoftWhare Mar 18, 2022 •

edited

Loading

SBiOSoftWhare Mar 18, 2022

SBiOSoftWhare Mar 18, 2022

SBiOSoftWhare Mar 18, 2022

SBiOSoftWhare Mar 23, 2022

SBiOSoftWhare Mar 23, 2022

SBiOSoftWhare Mar 23, 2022

SBiOSoftWhare Mar 23, 2022

SBiOSoftWhare Mar 23, 2022

SBiOSoftWhare Mar 23, 2022

SBiOSoftWhare Mar 23, 2022

MaximeEvrard42 Mar 24, 2022

		@@ -20,14 +20,23 @@ import Mapbox

		class LocationMarkerView: MGLAnnotationView, NibLoadable {

		@IBOutlet private var markerBackground: UIImageView!

	@IBOutlet private var markerBackground: UIImageView!
	@IBOutlet private var backgroundImageView: UIImageView!

+@available(iOS 14.0, *)
+extension AvatarImage {
+ func border(color: Color) -> some View {
+ modifier(BorderModifier(color: color, borderWidth: 3, shape: Circle()))
+ }
+ /// Use display name color as border color by default
+ func border() -> some View {
+ let borderColor = theme.displayNameColor(for: matrixItemId)
+ return self.border(color: borderColor)
+ }
+}

Update UI in location sharing view #5720 #5859

Update UI in location sharing view #5720 #5859

Conversation

MaximeEvrard42 commented Mar 17, 2022 • edited Loading

github-actions bot commented Mar 17, 2022 • edited Loading

SBiOSoftWhare left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SBiOSoftWhare Mar 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaximeEvrard42 commented Mar 17, 2022 •

edited

Loading

github-actions bot commented Mar 17, 2022 •

edited

Loading

SBiOSoftWhare Mar 18, 2022 •

edited

Loading