Skip to content
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

Conversation

khyativyasargus
Copy link
Contributor

@khyativyasargus khyativyasargus commented Apr 24, 2023

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

Checklist

  • I have read and acknowledged the Code of conduct.
  • I have read the Contributing page.
  • I have signed the Google Individual CLA, or I am covered by my company's Corporate CLA.
  • I have discussed my proposed solution with code owners in the linked issue(s) and we have agreed upon the general approach.
  • I have run ./gradlew spotlessApply and ./gradlew spotlessCheck to check my code follows the style guide of this project.
  • I have run ./gradlew check and ./gradlew connectedCheck to test my changes locally.
  • I have built and run the demo app(s) to verify my change fixes the issue and/or does not break the demo app(s).

@santosh-pingle
Copy link
Collaborator

@khyativyasargus
I noticed a slight cut on the right side of the help button. Could you please confirm this?

Can you also try the following precondition and see how the help button aligns?
Please modify questionnaire.json and add 1. precondition text 2. long question text

@khyativyasargus
Copy link
Contributor Author

Thanks @santosh-pingle for drawing my attention. Sure will check and provide requested details. Just one doubt can you elaborate about precondition here?

@santosh-pingle
Copy link
Collaborator

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 "prefix":"3." and "text": "Adding long text to just see how help button get aligns, its just testing.Adding long text to just see how help button get aligns, its just testing.Adding long text to just see how help button get aligns, its just testing",

@khyativyasargus
Copy link
Contributor Author

I noticed a slight cut on the right side of the help button. Could you please confirm this?

@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.
so to solve that there are 2 options in my mind right now,
1)increase button width and height by 1dp and put 1dp padding
2)change widget type from Button to ImageView

what do you suggest? Let me know if you have any other option on your mind , I'll do changes accordingly.

@khyativyasargus
Copy link
Contributor Author

khyativyasargus commented Apr 27, 2023

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 "prefix":"3." and "text": "Adding long text to just see how help button get aligns, its just testing.Adding long text to just see how help button get aligns, its just testing.Adding long text to just see how help button get aligns, its just testing",

@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

image

image

image

@santosh-pingle
Copy link
Collaborator

santosh-pingle commented May 3, 2023

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 "prefix":"3."and"text": "Adding long text to just see how help button get aligns, its just testing.Adding long text to just see how help button get aligns, its just testing.Adding long text to just see how help button get aligns, its just testing",`

@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

image

image

image

@shelaghm Can you please review attached screenshots by @khyativyasargus

@khyativyasargus, you mean, changes will be updated to this pr, correct?

@khyativyasargus
Copy link
Contributor Author

@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.

@jingtang10
Copy link
Collaborator

@jingtang10
Copy link
Collaborator

@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?

@khyativyasargus
Copy link
Contributor Author

khyativyasargus commented May 4, 2023

@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.

@khyativyasargus
Copy link
Contributor Author

khyativyasargus commented May 10, 2023

@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?

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.
But I think Text & Icon within spannable should also be customized with custom style attributes and I am not sure how we can achieve that. Can you guide me about the possibility?

@jingtang10
Copy link
Collaborator

hey @khyativyasargus thanks for the update. can you add new screenshots, especially for long titles?

@khyativyasargus
Copy link
Contributor Author

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

image

@shelaghm
Copy link

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 Screenshot 2023-05-17 at 08 04 50

@khyativyasargus khyativyasargus requested a review from a team as a code owner July 7, 2023 15:53
Copy link
Collaborator

@aditya-07 aditya-07 left a 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.

Comment on lines +106 to +108
fun setSpan(clickableSpan: ClickableSpan) {

}
Copy link
Collaborator

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.

Comment on lines +129 to +134
// helpButton.visibility =
// if (questionnaireItem.hasHelpButton) {
// VISIBLE
// } else {
// GONE
// }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove it.

Comment on lines +52 to +57
// 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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove it.

Comment on lines +50 to +104
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)
}
Copy link
Collaborator

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?

@aditya-07
Copy link
Collaborator

@khyativyasargus Please update the PR to latest changes.

@khyativyasargus
Copy link
Contributor Author

If we are not depending on any kotlin code to place the Help Button, then we should remove it from this PR.

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.

@jingtang10
Copy link
Collaborator

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.

@jingtang10 jingtang10 closed this Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

[SDC] Move help button location to after question title
5 participants