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

Allow modification of Questionnaire submit action button text via configuration #2319

Merged
merged 20 commits into from
Jan 31, 2024

Conversation

DebbieArita
Copy link
Contributor

@DebbieArita DebbieArita commented Oct 27, 2023

…figuration

IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).

Fixes #2239

Description
Clear and concise code change description.

Alternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?

Type
Choose one: (Bug fix | Feature | Documentation | Testing | Code health | Builds | Releases | Other)

Screenshots (if applicable)

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

@google-cla
Copy link

google-cla bot commented Oct 27, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Collaborator

@MJ1998 MJ1998 left a comment

Choose a reason for hiding this comment

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

LGTM

@DebbieArita
Copy link
Contributor Author

@santosh-pingle @MJ1998 could you help me resolve the Kokoro check and maybe point out anything else I need to close on this?

@MJ1998
Copy link
Collaborator

MJ1998 commented Nov 16, 2023

@DebbieArita
Kokoro is running right now. Let's wait for the result.

@DebbieArita
Copy link
Contributor Author

@santosh-pingle @MJ1998 could you help me resolve the Kokoro check and maybe point out anything else I need to close on this?

@DebbieArita Kokoro is running right now. Let's wait for the result.

Alright, thank you.

Copy link
Collaborator

@jingtang10 jingtang10 left a comment

Choose a reason for hiding this comment

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

Please try to add this line:

    <string name="submit_questionnaire">New Submit String</string>

to the resource files in your app, and I think it'll override the string value in the library resources.

and if you really need to do this at run-time (which I'm not too keen on actually), you can do it this way: https://blog.kiprosh.com/how-to-replace-strings-xml-dynamically-in-android/

Please try this and see if this works better for you. My concern here is that if we provide an API / configuration for each string we'll end up with a ton of new API surfaces... and I worry that we'll struggle to manage.

@ellykits
Copy link
Collaborator

ellykits commented Dec 19, 2023

@jingtang10 Changing the text via the strings.xml file would be the easiest however, this would change the text through the application which is not desired in our current use case. We'd want to change the text only for particular questionnaires.

cc @DebbieArita

@ellykits
Copy link
Collaborator

ellykits commented Dec 19, 2023

and if you really need to do this at run-time (which I'm not too keen on actually), you can do it this way: https://blog.kiprosh.com/how-to-replace-strings-xml-dynamically-in-android/

Please try this and see if this works better for you. My concern here is that if we provide an API / configuration for each string we'll end up with a ton of new API surfaces... and I worry that we'll struggle to manage.

I missed reading this part of your comment @jingtang10. We only required the update for the questionnaire submission text. We'll try the suggestion. From the reference shared I do not think this will address a scenario where we would want to use 2 different texts for a questionnaire submission text.

Copy link
Collaborator

@jingtang10 jingtang10 left a comment

Choose a reason for hiding this comment

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

i think this change alone is fine. but let's be careful with introducing too many arguments in the future.

@jingtang10 jingtang10 enabled auto-merge (squash) January 30, 2024 17:40
@jingtang10 jingtang10 merged commit 4d675da into google:master Jan 31, 2024
3 checks passed
@jingtang10 jingtang10 deleted the modify-submit branch January 31, 2024 16:07
sharon2719 pushed a commit to opensrp/android-fhir that referenced this pull request Feb 6, 2024
…figuration (google#2319)

* Allow modification of Questionnaire submit action button text via configuration

* Run spotlessApply

* Update test to check that button text is editable

* Update test to check that button text is editable

* 🎨 Apply Spotless formatting

* Empty commit

* Resolve merge conflicts

---------

Co-authored-by: Allan Onchuru <a.onchuru@gmail.com>
Co-authored-by: Francis Odhiambo <4540684+f-odhiambo@users.noreply.github.com>
Co-authored-by: Benjamin Mwalimu <dubdabasoduba@gmail.com>
Co-authored-by: Allan Onchuru <16164649+allan-on@users.noreply.github.com>
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.

Allow modification of Questionnaire submit action button text via configuration
9 participants