-
Notifications
You must be signed in to change notification settings - Fork 305
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
Placed help button location to after question title #1984
Placed help button location to after question title #1984
Conversation
@khyativyasargus Can you also try the following precondition and see how the help button aligns? |
Thanks @santosh-pingle for drawing my attention. Sure will check and provide requested details. Just one doubt can you elaborate about precondition here? |
Actually, i just wanted to see if prefix text is present and question text is 2-3 line text in the json file then how help button get aligns`. In the component_help.json, you can add |
@santosh-pingle This is because for help button, Button widget is used which is ideally in square shape with edges and we are putting round shape image into that. what do you suggest? Let me know if you have any other option on your mind , I'll do changes accordingly. |
@santosh-pingle I've done some corrections to manage long text, but I'll commit them with help button changes together. Here it is SS for how it looks |
@shelaghm Can you please review attached screenshots by @khyativyasargus @khyativyasargus, you mean, changes will be updated to this pr, correct? |
Yes @santosh-pingle , once you & @shelaghm approves screenshots I'll commit changes in this PR only. |
@khyativyasargus thanks for this change, would it be possible to also refactor the header so that the prefix is also inline the same way the icon is inline in the link i posted above? |
Yes @jingtang10 this option sounds more convenient to me as per UI perspective. so you want prefix & help icon both inline with title right? I'll give it check and will update you with changes. |
Hi @jingtang10 @santosh-pingle ! As per our conversation, I've done with refactoring the question header view in such a way that the Prefix, Title, and icon stay inline with the help of Spannable. |
hey @khyativyasargus thanks for the update. can you add new screenshots, especially for long titles? |
Sure @jingtang10 , Here is a screenshot of inline headerview with longer title |
Thanks @khyativyasargus Looks better, one small adjustment would be to make the help button aligned to the text line height? See screenshot as an example |
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.
If we are not depending on any kotlin code to place the Help Button, then we should remove it from this PR.
fun setSpan(clickableSpan: ClickableSpan) { | ||
|
||
} |
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 it if not required.
// helpButton.visibility = | ||
// if (questionnaireItem.hasHelpButton) { | ||
// VISIBLE | ||
// } else { | ||
// GONE | ||
// } |
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 it.
// question.setIconifiedText(questionnaireViewItem.questionnaireItem.localizedPrefixSpanned , | ||
// questionnaireViewItem.questionnaireItem.localizedTextSpanned, | ||
// R.drawable.ic_help_48px, | ||
// helpCardView = findViewById(R.id.helpCardView), | ||
// helpTextView = findViewById(R.id.helpText), | ||
// questionnaireItem = questionnaireViewItem.questionnaireItem) |
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 it.
internal fun TextView.setIconifiedText(localizedPrefix: Spanned? = null, | ||
localizedText: Spanned? = null, @DrawableRes iconResId: Int, | ||
helpCardView: MaterialCardView, | ||
helpTextView: TextView, | ||
questionnaireItem: Questionnaire.QuestionnaireItemComponent) { | ||
text = if(!localizedPrefix.isNullOrEmpty() && !localizedText.isNullOrEmpty()) | ||
TextUtils.concat(localizedPrefix, localizedText) | ||
else localizedText | ||
visibility = | ||
if (text.isNullOrEmpty() && !questionnaireItem.hasHelpButton) { | ||
GONE | ||
} else { | ||
VISIBLE | ||
} | ||
val clickableSpan = object : ClickableSpan() { | ||
override fun onClick(widget: View) { | ||
helpCardView.visibility = | ||
when (helpCardView.visibility) { | ||
VISIBLE -> GONE | ||
else -> VISIBLE | ||
} | ||
} | ||
} | ||
SpannableStringBuilder("$text#").apply { | ||
val attrs = context.obtainStyledAttributes(intArrayOf(R.attr.questionnaireHelpButtonStyle)) | ||
val helpButtonStyleResId = attrs.getResourceId(0, 0) | ||
|
||
// Retrieve the color defined in questionnaireHelpButtonStyle attribute | ||
val helpButtonStyleAttrs = context.obtainStyledAttributes(helpButtonStyleResId, intArrayOf(R.attr.tint)) | ||
val helpButtonColor = helpButtonStyleAttrs.getColor(0, Color.BLACK) | ||
|
||
// Create a new ImageSpan and apply the color to it | ||
ContextCompat.getDrawable(context, R.drawable.ic_help_48px).apply { | ||
this!!.setTint(helpButtonColor) | ||
} | ||
val imageSpan = ImageSpan(context,R.drawable.ic_help_48px, DynamicDrawableSpan.ALIGN_CENTER) | ||
|
||
setSpan( | ||
imageSpan, | ||
text.length, | ||
text.length + 1, | ||
Spanned.SPAN_EXCLUSIVE_EXCLUSIVE | ||
) | ||
setSpan(clickableSpan, | ||
text.length, | ||
text.length + 1, | ||
Spanned.SPAN_EXCLUSIVE_EXCLUSIVE | ||
) | ||
}.let { | ||
text = it | ||
movementMethod = LinkMovementMethod.getInstance() | ||
} | ||
|
||
helpTextView.updateTextAndVisibility(questionnaireItem.localizedHelpSpanned) | ||
} |
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.
Can we remove this code if it is not being used anywhere?
@khyativyasargus Please update the PR to latest changes. |
Hi @aditya-07 these changes are required but as we haven't been able to find a way to configure Image using style attribute with Spannable so it's still pending. We can refactor the code of this PR once we find the final solution. |
Given the status of this PR I'm going to close this for now until we have a proper solution that works well. @khyativyasargus if this becomes blocking please feel free to reopen. |
IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).
Fixes #1965
Description
Move help button location to after question title as subtitle might be empty in some cases.
Alternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?
Type
Choose one: Feature
Screenshots (if applicable)
![image](https://user-images.githubusercontent.com/121015652/233987172-970f5670-94f1-41f6-9bb9-3074b0df7e48.png)
Checklist
./gradlew spotlessApply
and./gradlew spotlessCheck
to check my code follows the style guide of this project../gradlew check
and./gradlew connectedCheck
to test my changes locally.