-
Notifications
You must be signed in to change notification settings - Fork 75
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
[MOB-7458] - Aligning iOS with Android #704
[MOB-7458] - Aligning iOS with Android #704
Conversation
Config object Enum for type Removal of ViewDelegate and its methods
1. Clicks now call the action handler and urlHandler of iterableConfig. This means they can open passed url. However, in recent commit, Ive removed the generic delegate methods which removes the didPrimaryButtonPressed and similar callbacks that viewControllers could implement. Now this is more aligned with Android implementation. 2. Sync messages now gets called in manager's init method. We have to try accomplishing the same thing via app's lifecycle activity. Somehow, for the first launch, the manager is not calling sync messages. Hence calling it during initialization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM at least for today's bug bash.
A few points we need to address before release:
- notification default is not aligned with Android. The background and border colors differ from the card and banner styles.
- the second button is defaulting to this weird block button. it should be a button with a transparent background and the same color text as the first button. See the figma.
- the second button background and border color should be configurable. I will add this to the android side.
- the card view type should collapse when the image is not provided to align with the android side and the discussion I had with design (stephanie)
|
||
public init(backgroundColor: UIColor? = UIColor.white, | ||
borderColor: UIColor? = UIColor.white, | ||
borderWidth: CGFloat? = 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. But the notification view has different characteristics.
} | ||
} | ||
|
||
//TODO: Remove commented lines before release |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove commented lines.
Will bug bash with this branch so we can work on this before merging into embedded |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. merging with base feature branch.
🔹 Jira Ticket(s)
✏️ Description
Config object
Enum for type
Removal of ViewDelegate and its methods