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

Remove extra space from a Questionnaire View when the group headers have no text to show. #1339

Merged
merged 5 commits into from
Apr 28, 2022

Conversation

aditya-07
Copy link
Collaborator

@aditya-07 aditya-07 commented Apr 27, 2022

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

Fixes #[issue number]

Description

  1. If there is no text to be displayed in QuestionnaireItemHeaderView i.e. No prefix, question or hint, then set the visibility of the View as GONE. Otherwise there will be blank space.
  1. Removed ErrorView from QuestionnaireItemGroupViewHolder as the group header won't have any errors to show.

Based on Jing's comment below, error can be shown in the QuestionnaireItemGroupViewHolder. So, keeping ErrorView.

Alternative(s) considered
The alternative would be to weed out such QuestionnaireItemComponent while processing Questionnaire so that they are not in the list provided to the RecyclerView.

Type
Choose one: Bug fix

Screenshots (if applicable)
Black lines here are the DividerItemDecoration to display the rows in the recyclerview.

Before:
Screenshot_with_empty_group_header_visible

After:
Screenshot_with_empty_group_header_gone

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

@codecov
Copy link

codecov bot commented Apr 28, 2022

Codecov Report

Merging #1339 (9a14627) into master (13aa710) will increase coverage by 0.00%.
The diff coverage is 85.71%.

@@            Coverage Diff            @@
##             master    #1339   +/-   ##
=========================================
  Coverage     84.57%   84.57%           
- Complexity      687      690    +3     
=========================================
  Files           148      148           
  Lines         10701    10708    +7     
  Branches        832      836    +4     
=========================================
+ Hits           9050     9056    +6     
  Misses         1237     1237           
- Partials        414      415    +1     
Impacted Files Coverage Δ
...e/views/QuestionnaireItemGroupViewHolderFactory.kt 75.00% <0.00%> (-5.00%) ⬇️
...r/datacapture/views/QuestionnaireItemHeaderView.kt 90.90% <100.00%> (+2.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6f4a654...9a14627. Read the comment docs.

@aditya-07 aditya-07 enabled auto-merge (squash) April 28, 2022 18:20
@@ -43,6 +43,8 @@ internal object QuestionnaireItemGroupViewHolderFactory :
error.text =
if (validationResult.getSingleStringValidationMessage() == "") null
else validationResult.getSingleStringValidationMessage()

error.visibility = if (error.text.isNotEmpty()) View.VISIBLE else View.GONE
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you use isNullOrEmpty?

Copy link
Collaborator

Choose a reason for hiding this comment

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

discussed with @aditya-07 offline - text view always returns non-null string, so this should be good.

@aditya-07 aditya-07 merged commit 7251ddf into google:master Apr 28, 2022
@aditya-07 aditya-07 deleted the ak/hide-groupheaderview-when-empty branch April 19, 2023 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants