Apply Unified Customization to Persistent Button #687
Conversation
rasmustautsglia
requested review from
igorkravchenko,
gersonnoboa,
EgorovEI and
yurii-glia
| let gvaStyle = gvaStyle | ||
|
|
There was a problem hiding this comment.
could we pass gvaStyle directly to ChatStyle without this local variable?
There was a problem hiding this comment.
fixed 52e93dc
| import UIKit | ||
| extension Theme { |
There was a problem hiding this comment.
| import UIKit | |
| extension Theme { | |
| import UIKit | |
| extension Theme { |
There was a problem hiding this comment.
fixed 52e93dc
| public var systemMessage: SystemMessageStyle | ||
|
|
||
| /// Style of the GVA |
There was a problem hiding this comment.
I'd like to propose using full name of the feature instead of GVA, would it make sense?
There was a problem hiding this comment.
Do you mean in this descriptive line or in general gva -> Glia Virtual Assistant?
There was a problem hiding this comment.
I would also like if it says Glia Virtual Assistant, especially here that it's a comment to help the future reader.
There was a problem hiding this comment.
fixed a4db754
|
!squash |
sm-deployer
force-pushed
the
Apply-Unified-Customization-to-persistent-button
branch
from
a4db754
to
e1bd634
Compare
| isa = PBXGroup; | ||
| children = ( | ||
| C0175A162A5D30D7001FACDE /* GvaResponseTextView.swift */, | ||
| C0175A222A65614E001FACDE /* GVAPersistentButtonView.swift */, |
There was a problem hiding this comment.
I think files need to be called Gva... rather than GVA... because of some issue with Swift Package Manager or Xcfs or something like that. Same with the Theme+Gva
There was a problem hiding this comment.
changed 1128e9a
| backgroundColor: .clear | ||
| ), | ||
| backgroundColor: .fill(color: color.lightGrey), | ||
| cornerRadius: 8.49, |
There was a problem hiding this comment.
I wonder why we have these super precise corner radiuses? :D I guess they are part of Figma, but perhaps in other places we do them with round numbers?
There was a problem hiding this comment.
I will ask about it from Aksel
There was a problem hiding this comment.
If I'm not mistaken, cornerRadius for PB is 5, and radius for the card is 8
There was a problem hiding this comment.
I honestly don't know, because I was relying on this section instead
There was a problem hiding this comment.
I would not suggest to rely on Code section at all)
Usually it suggests useless awful code)
There was a problem hiding this comment.
Temporary fix is 10px as disgussed with Yaro. Changed in 2596c15
| @@ -90,7 +94,8 @@ public class ChatStyle: EngagementStyle { | |||
| secureTranscriptTitle: String, | |||
| secureTranscriptHeader: HeaderStyle, | |||
| unreadMessageDivider: UnreadMessageDividerStyle, | |||
| systemMessage: SystemMessageStyle | |||
| systemMessage: SystemMessageStyle, | |||
| gva: GvaStyle | |||
There was a problem hiding this comment.
I am worried about this for the future. If someone else comes and reads gva, they will not understand. However having this called gliaVirtualAssistantStyle is long-ish. Don't know exactly what to do.
There was a problem hiding this comment.
I think it is indeed more descriptive. I will change it
There was a problem hiding this comment.
changed 1128e9a
| @@ -9,7 +9,7 @@ final class GvaPersistentButtonView: OperatorChatMessageView { | |||
| private let environment: Environment | |||
|
|
|||
| init(with style: ChatStyle, environment: Environment) { | |||
| self.viewStyle = style.gva.persistentButton | |||
| self.viewStyle = style.gliaVirtualAssistant.persistentButton | |||
There was a problem hiding this comment.
There's a SwiftLint issue on this file, at the end
There was a problem hiding this comment.
fixed c853494
|
!squash |
This PR adds Unified customization to Persistent button, together with default theme. In GVAPersistentButtonVIew the ContainerView was moved to a stand-alone class so layoutSubviews() could be used properly, since gradient color can only be applied in that method. In addition, it adds system message to unified customization since it wasn't applied before but was actually ready to be used. MOB-2372
sm-deployer
force-pushed
the
Apply-Unified-Customization-to-persistent-button
branch
from
2596c15
to
2f32a34
Compare
rasmustautsglia
deleted the
Apply-Unified-Customization-to-persistent-button
branch
This PR adds Unified customization to Persistent button, together with default theme. In GVAPersistentButtonVIew the ContainerView was moved to a stand-alone class so layoutSubviews() could be used properly, since gradient color can only be applied in that method.
In addition, it adds system message to unified customization since it wasn't applied before but was actually ready to be used.
MOB-2372