Fixes main content padding on various screens #2906

brandyodhiambo · 2023-12-04T15:51:58Z

IMPORTANT: Where possible all PRs must be linked to a Github issue

Engineer Checklist

I have written Unit tests for any new feature(s) and edge cases for bug fixes
I have added any strings visible on UI components to the strings.xml file
I have updated the CHANGELOG.md file for any notable changes to the codebase
I have run ./gradlew spotlessApply and ./gradlew spotlessCheck to check my code follows the project's style guide
I have built and run the FHIRCore app to verify my change fixes the issue and/or does not break the app
I have checked that this PR does NOT introduce breaking changes that require an update to Content and/or Configs? If it does add a sample here or a link to exactly what changes need to be made to the content.

Code Reviewer Checklist

I have verified Unit tests have been written for any new feature(s) and edge cases
I have verified any strings visible on UI components are in the strings.xml file
I have verifed the CHANGELOG.md file has any notable changes to the codebase
I have verified the solution has been implemented in a configurable and generic way for reuseable components
I have built and run the FHIRCore app to verify the change fixes the issue and/or does not break the app

Demo

Image without FAB	Image With FAB

…eens

codecov · 2023-12-04T16:27:50Z

Codecov Report

Merging #2906 (2552ac3) into main (ad3a737) will increase coverage by 2.2%.
Report is 114 commits behind head on main.
The diff coverage is 63.7%.

Additional details and impacted files

@@            Coverage Diff            @@
##              main   #2906     +/-   ##
=========================================
+ Coverage     64.5%   66.8%   +2.2%     
- Complexity    1075    1092     +17     
=========================================
  Files          218     222      +4     
  Lines         9635   10162    +527     
  Branches      1897    1879     -18     
=========================================
+ Hits          6218    6790    +572     
+ Misses        2234    2183     -51     
- Partials      1183    1189      +6

Flag	Coverage Δ
engine	`70.4% <67.8%> (-2.3%)`	⬇️
geowidget	`65.5% <33.3%> (+1.1%)`	⬆️
quest	`64.4% <61.0%> (+5.4%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...ngine/configuration/view/CompoundTextProperties.kt	`100.0% <ø> (ø)`
...gine/configuration/workflow/ApplicationWorkflow.kt	`100.0% <100.0%> (ø)`
...r/fhircore/engine/domain/model/RoundingStrategy.kt	`100.0% <100.0%> (ø)`
...gister/fhircore/engine/rulesengine/RulesFactory.kt	`71.8% <100.0%> (+0.6%)`	⬆️
...martregister/fhircore/engine/sync/AppSyncWorker.kt	`100.0% <ø> (ø)`
...r/fhircore/engine/task/FhirResourceExpireWorker.kt	`100.0% <100.0%> (ø)`
...fhircore/engine/task/FhirTaskStatusUpdateWorker.kt	`100.0% <100.0%> (ø)`
...g/smartregister/fhircore/engine/ui/theme/Colors.kt	`100.0% <100.0%> (ø)`
...rcore/engine/util/extension/FhirEngineExtension.kt	`80.0% <100.0%> (+2.8%)`	⬆️
...fhircore/engine/util/extension/StringExtensions.kt	`91.6% <100.0%> (ø)`
... and 77 more

... and 29 files with indirect coverage changes

Rkareko · 2023-12-06T06:37:09Z

...ine/src/main/java/org/smartregister/fhircore/engine/ui/components/register/RegisterFooter.kt

+        modifier
+          .fillMaxWidth()
+          .testTag(SEARCH_FOOTER_TAG)
+          .padding(bottom = if (!fabActions.isNullOrEmpty() && fabActions.first().visible) 80.dp else 32.dp)


Convert these to constants with intuitive names e.g FOOTER_PADDING_WITH_FAB, FOOTER_PADDING_WITHOUT_FAB

it should be a constant variable? for both

See my suggested code in the comments. The idea is to define the padding values in 2 separate constants to be reused in all screens.

Rkareko · 2023-12-06T06:37:28Z

android/quest/src/main/java/org/smartregister/fhircore/quest/ui/profile/ProfileScreen.kt

-      LazyColumn(state = lazyListState) {
+      LazyColumn(
+        state = lazyListState,
+        modifier = Modifier.padding(bottom = if (!fabActions.isNullOrEmpty() && fabActions.first().visible) 80.dp else 32.dp)


Convert these to constants with intuitive names e.g FOOTER_PADDING_WITH_FAB, FOOTER_PADDING_WITHOUT_FAB

ellykits · 2023-12-06T08:17:09Z

@brandy-kay Can you link the issue to this PR?

ellykits · 2023-12-06T08:23:22Z

...ine/src/main/java/org/smartregister/fhircore/engine/ui/components/register/RegisterFooter.kt

+          .testTag(SEARCH_FOOTER_TAG)
+          .padding(
+            bottom =
+              if (!fabActions.isNullOrEmpty() && fabActions.first().visible) 80.dp else 32.dp,


Suggested change

if (!fabActions.isNullOrEmpty() && fabActions.first().visible) 80.dp else 32.dp,

//Declared these constants in AndroidExtensions file

const val PADDING_BOTTOM_WITH_FAB = 80

const val PADDING_BOTTOM_WITHOUT_FAB = 32

if (!fabActions.isNullOrEmpty() && fabActions.first().visible) PADDING_BOTTOM_WITH_FAB.dp else PADDING_BOTTOM_WITHOUT_FAB.dp,

ellykits · 2023-12-06T08:25:36Z

Kindly can you share screenshots to verify this is fixed on the UI?

brandyodhiambo · 2023-12-06T09:34:01Z

@ellykits Sure the PR is WIP once done I will Linking all the necessary UI let me include the changes

brandyodhiambo · 2023-12-06T12:51:18Z

@Rkareko and @ellykits the pr is now ready for review

brandyodhiambo added 3 commits December 4, 2023 11:18

adds padding to the profile lazy column

bd89d3d

Merge branch 'main' into 2892-fix-main-content-padding-on-various-scr…

bbca399

…eens

setting padding on Register footer row ,on measure report list screen.

8568e31

brandyodhiambo linked an issue Dec 4, 2023 that may be closed by this pull request

Fix main content padding on various screens #2892

Open

3 tasks

brandyodhiambo added 2 commits December 5, 2023 16:39

fixes MeasureReport List Screen

8a5f890

runs profile screen test

f134986

Rkareko reviewed Dec 6, 2023

View reviewed changes

brandyodhiambo added 2 commits December 6, 2023 10:04

runs engine test

34599c0

runs spotless

f5a50b9

ellykits requested changes Dec 6, 2023

View reviewed changes

brandyodhiambo added 4 commits December 6, 2023 14:17

edits the constant paddings on profile screen and register footer

017decc

edits the constant paddings on profile screen and register footer

f3fb14f

runs quest spotless

0467433

runs engine spotless

2552ac3

brandyodhiambo marked this pull request as ready for review December 6, 2023 12:49

ellykits approved these changes Dec 6, 2023

View reviewed changes

f-odhiambo merged commit 3cbb77b into main Dec 6, 2023
6 checks passed

f-odhiambo deleted the 2892-fix-main-content-padding-on-various-screens branch December 6, 2023 13:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes main content padding on various screens #2906

Fixes main content padding on various screens #2906

brandyodhiambo commented Dec 4, 2023 •

edited

Loading

codecov bot commented Dec 4, 2023 •

edited

Loading

Rkareko Dec 6, 2023

brandyodhiambo Dec 6, 2023

ellykits Dec 6, 2023

Rkareko Dec 6, 2023

ellykits commented Dec 6, 2023

ellykits Dec 6, 2023

ellykits commented Dec 6, 2023

brandyodhiambo commented Dec 6, 2023

brandyodhiambo commented Dec 6, 2023

-              if (!fabActions.isNullOrEmpty() && fabActions.first().visible) 80.dp else 32.dp,
+//Declared these constants in  AndroidExtensions file
+const val PADDING_BOTTOM_WITH_FAB = 80
+const val PADDING_BOTTOM_WITHOUT_FAB = 32
+              if (!fabActions.isNullOrEmpty() && fabActions.first().visible) PADDING_BOTTOM_WITH_FAB.dp else PADDING_BOTTOM_WITHOUT_FAB.dp,

Fixes main content padding on various screens #2906

Fixes main content padding on various screens #2906

Conversation

brandyodhiambo commented Dec 4, 2023 • edited Loading

Demo

codecov bot commented Dec 4, 2023 • edited Loading

Codecov Report

Rkareko Dec 6, 2023

Choose a reason for hiding this comment

brandyodhiambo Dec 6, 2023

Choose a reason for hiding this comment

ellykits Dec 6, 2023

Choose a reason for hiding this comment

Rkareko Dec 6, 2023

Choose a reason for hiding this comment

ellykits commented Dec 6, 2023

ellykits Dec 6, 2023

Choose a reason for hiding this comment

ellykits commented Dec 6, 2023

brandyodhiambo commented Dec 6, 2023

brandyodhiambo commented Dec 6, 2023

brandyodhiambo commented Dec 4, 2023 •

edited

Loading

codecov bot commented Dec 4, 2023 •

edited

Loading