Add FXIOS-9109 [Component Library] Disabled state for primary button #20205
Conversation
cyndichin
force-pushed
the
cc/FXIOS-9109_update-disable-state-for-primary-button
branch
from
d770fab
to
bc6b676
Compare
|
@isabelrios @clarmso it seems that Deploy Docc step always fails on individual PRs, but when I check the history it passes when merged into main. Is this something you have more context on? It seems we can merge regardless. |
Generated by 馃毇 Danger Swift against bc6b676 |
I'm not really familiar with that step, not sure who owns / maintains it. I can take a look to try to fix it while we ask about the owner and what is the expected / the reason of that step... |
| @@ -52,6 +53,13 @@ class ButtonsViewController: UIViewController, Themeable { | |||
| a11yIdentifier: "a11yPrimary") | |||
| primaryButton.configure(viewModel: primaryViewModel) | |||
|
|
|||
| let primaryViewModelDisabled = PrimaryRoundedButtonViewModel( | |||
| title: "Primary Disabled", | |||
There was a problem hiding this comment.
| title: "Primary Disabled", | |
| title: "Primary Disabled", |
do we need to make these strings go inside strings files instead of directly adding the string here?
There was a problem hiding this comment.
adding here as this is part of the sample component library
| if self?.state == .disabled { | ||
| container.foregroundColor = self?.foregroundColorDisabled | ||
| } else { | ||
| container.foregroundColor = self?.foregroundColor | ||
| } |
There was a problem hiding this comment.
On one side I like this because its readable and on another side I feel ternary operator works well for clean approach
container.foregroundColor = self?.state == .disabled ? self?.foregroundColorDisabled : self?.foregroundColor
There was a problem hiding this comment.
agreed, I also prefer ternary. discussed live, but will keep this as is since easier to read
|
thanks for reviewing @nbhasin2! |
cyndichin
deleted the
cc/FXIOS-9109_update-disable-state-for-primary-button
branch
馃摐 Tickets
Jira ticket
Github issue
馃挕 Description
Update our component
PrimaryRoundedButtonto update its color based on its disabled state.Added two new tokens
actionPrimaryDisabledandtextInvertedDisabled馃摑 Checklist
You have to check all boxes before merging
@Mergifyio backport release/v120)Screenshots