-
Notifications
You must be signed in to change notification settings - Fork 169
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
#602 Change post button to be more obvious #691
#602 Change post button to be more obvious #691
Conversation
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.
Your change looks great. Can you fix the commit history?
e705b43
to
d05bd81
Compare
Run |
d05bd81
to
6d9345a
Compare
contentDescription = stringResource(R.string.form_submit), | ||
) | ||
// Todo add are you sure cancel dialog+ | ||
if (formValid && !loading) { |
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.
Loading was already checked above.
// Todo add are you sure cancel dialog+ | ||
if (formValid && !loading) { | ||
Icon( | ||
Icons.Filled.Send, |
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.
Would be much easier to do icon = if (formValid) { ...
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.
Yep, that's much cleaner
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. I'll handle the merge conflict.
resolves #602 This changes the submit icon from
add
tosubmit
and adds a preview for the post header.