-
Notifications
You must be signed in to change notification settings - Fork 59
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
Enhance Image and ActionableButton Widget to add ability to copy text #3040
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.
👍 LGTM. Just a simple comment on the test case.
composeRule | ||
.onNodeWithText("Copy Button Text", useUnmergedTree = true) | ||
.assertExists() | ||
.assertIsDisplayed() | ||
.performClick() |
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.
Should there be a verification done for this perform click action?
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.
I will update the test to test on click function instead
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #3040 +/- ##
==========================================
- Coverage 64.5% 29.8% -34.8%
+ Complexity 1075 652 -423
==========================================
Files 218 236 +18
Lines 9635 11088 +1453
Branches 1897 1935 +38
==========================================
- Hits 6218 3307 -2911
- Misses 2234 7350 +5116
+ Partials 1183 431 -752
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…srp/fhircore into fix_issue_3033_add_copy_text_widget
@MargaretNjenga @f-odhiambo , adding the link to be copied as text might jump to 3 lines or more and would not have a better UI/UX. However, we can add a text description instead and then a copy icon Then a toast shows when one clicks on the icon, below message appears Kindly advise if there's anything needed to change as I fix failing tests |
@SebaMutuku Can we add the 'Tap icon ...' text as normal text and not make it look like a hyperlink? Rest of the stuff LGTM |
@f-odhiambo This has been updated |
@SebaMutuku Let's make the other text grey |
@f-odhiambo this is configurable. I only used that color for visibility |
@f-odhiambo @SebaMutuku I have marked this as blocked, it needs design review, fine to continue on functional requirements but it is not efficient to work on the design until @rowo and @HenryRae have reviewed, thank you |
@SebaMutuku @MargaretNjenga what are we trying to archive here with the copy link on the profile? What does this mean to getting consent from users first about sharing link? |
Here's a link to the proposed design. |
…srp/fhircore into fix_issue_3033_add_copy_text_widget
@HenryRae this has been effected. |
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.
looks good! @SebaMutuku can you update the docs to show how to add this copy text link to an app?
@pld updated. |
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 👍
Address the CI issues @SebaMutuku so we can merge this. |
* main: add docs on tagging (#3089) Constrain isAAB option to Boolean type (#3093) Create manual-apk-release.yml (#3003) Fix setting related entity location on Group member and related resources (#3092) Enhance Image and ActionableButton Widget to add ability to copy text (#3040) Add Related entity location tag on newly created resources via configs (#3086) Spotless clean Fixes config resources flagged for upsync 🐛 Clean up
* main: add docs on tagging (#3089) Constrain isAAB option to Boolean type (#3093) Create manual-apk-release.yml (#3003) Fix setting related entity location on Group member and related resources (#3092) Enhance Image and ActionableButton Widget to add ability to copy text (#3040) Add Related entity location tag on newly created resources via configs (#3086) Spotless clean Fixes config resources flagged for upsync 🐛 Clean up
IMPORTANT: Where possible all PRs must be linked to a Github issue
Fixes #3033
Engineer Checklist
strings.xml
file./gradlew spotlessApply
and./gradlew spotlessCheck
to check my code follows the project's style guideCode Reviewer Checklist
strings.xml
fileSample config for this