Set side nav padding and margin top in percentage

[jaypanchal-13]

Product Description

Ticket -> https://dimagi.atlassian.net/browse/CCCT-1776

Technical Summary

Set side nav margin and padding

ScreenShot ->
Small Device
left img -> before
right img-> after

Large Device
left img -> before
right img-> after

Feature Flag

Safety Assurance

Safety story

Automated test coverage

QA Plan

Labels and Review

• Do we need to enhance the manual QA test coverage ? If yes, the "QA Note" label is set correctly
• Does the PR introduce any major changes worth communicating ? If yes, the "Release Note" label is set and a "Release Note" is specified in PR description.
• Risk label is set correctly
• The set of people pinged as reviewers is appropriate for the level of risk of the change

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request adds dynamic padding and margin adjustments to the navigation drawer UI. It introduces a new public ID (nav_drawer_main_container) to the navigation drawer's root LinearLayout, adds a corresponding view reference in DrawerViewRefs, and implements logic in BaseDrawerController to compute the drawer's dimensions and apply responsive padding (4% of width) and margins (5% of height) to specific UI elements.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Focus areas:
  • Verification that dimension calculations (4% width padding and 5% height margin) are intentional and produce expected UI results
  • Confirmation that the post(Runnable) approach for deferred height calculation is appropriate and doesn't introduce timing issues
  • Validation that the padding/margin application on signoutView and navDrawerMainContainer doesn't conflict with existing layout constraints or cause unintended layout shifts

Possibly related PRs

• Side nav drawer in App Home page #3299: Directly modifies the same nav drawer implementation (BaseDrawerController.kt, DrawerViewRefs, and related layout IDs).
• CCCT-1720 App Sidebar Usability Improvement #3392: Modifies BaseDrawerController to extend drawer API functionality alongside this PR's view setup changes.
• CCCT-1775 Change Sidebar Account Setup Text To Button #3387: Updates the same navigation drawer layout (nav_drawer_base.xml) and setupViews logic for layout element handling.

Suggested labels

QA Note, skip-integration-tests

Suggested reviewers

• OrangeAndGreen
• shubham1g5
• conroy-ricketts
• Jignesh-dimagi

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check	⚠️ Warning	The PR description is incomplete. Critical sections including Safety story, Automated test coverage, and QA Plan are empty or contain only placeholder comments.	Complete the Safety Assurance section by describing how the change was tested, why it's safe, and provide a detailed QA plan. Also specify feature flag if applicable and address the checklist items.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and specifically summarizes the main change: setting side nav padding and margin in percentage-based values.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch CCCT-1776-side-nav-padding

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

• Provide your own instructions using the high_level_summary_instructions setting.
• Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
• Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
4. 📊 Contributor Summary — Include a Markdown table showing contributions:
   | Contributor | Lines Added | Lines Removed | Files Changed |
5. ✔️ Additional Notes — Add any extra reviewer context.
   Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

app/src/org/commcare/navdrawer/BaseDrawerController.kt (1)

5-5: Runtime padding/margin adjustments are implemented safely

Using nav_drawer_width for horizontal padding and drawerLayout.height for the signout top margin is consistent with the layout and avoids timing issues via post {}; the MarginLayoutParams cast is valid for the current hierarchy, so no crash risk here.

If you want to improve readability later, consider extracting the 0.04f and 0.05f literals into named constants (or dimen/fraction resources) to document the design intent and make future tuning easier.

Also applies to: 99-116

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 67c6ed1 and 5cfabfd.

📒 Files selected for processing (3)

• app/res/layout/nav_drawer_base.xml (1 hunks)
• app/src/org/commcare/navdrawer/BaseDrawerController.kt (2 hunks)
• app/src/org/commcare/navdrawer/DrawerViewRefs.kt (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Lint Code Base

🔇 Additional comments (2)

app/res/layout/nav_drawer_base.xml (1)

7-7: Root container id addition looks good

Exposing the root LinearLayout as nav_drawer_main_container is consistent with usage in DrawerViewRefs/BaseDrawerController and doesn’t introduce layout regressions.

app/src/org/commcare/navdrawer/DrawerViewRefs.kt (1)

28-28: New view reference is consistent with layout

navDrawerMainContainer correctly matches the new root layout id and follows the existing pattern of strongly-typed view refs.

[shubham1g5]

@jaypanchal-13 can you add before and after screenshots for this change.

Also instead of setting padding based on width percentage we can simply be using constant values based on different screen widths and define it in small screen resource classes

[jaypanchal-13]

@jaypanchal-13 can you add before and after screenshots for this change.

Also instead of setting padding based on width percentage we can simply be using constant values based on different screen widths and define it in small screen resource classes

@shubham1g5 Brandon suggested and in ticket it is mentioned for percentage approach. Updated PR description with after/before screenshot

[shubham1g5]

Ticket it is mentioned for percentage approach. Updated PR description with after/before screenshot

I think the percentage approach is less mainteneable (java instead of a simple xml constant value) and can be error prone as we add more views on the drawer. The outcome would be quite similar here in both aproached and it should not matter to product what approach we end up using based on technical maintenance and standard best practices for Android.

[jaypanchal-13]

Ticket it is mentioned for percentage approach. Updated PR description with after/before screenshot

I think the percentage approach is less mainteneable (java instead of a simple xml constant value) and can be error prone as we add more views on the drawer. The outcome would be quite similar here in both aproached and it should not matter to product what approach we end up using based on technical maintenance and standard best practices for Android.

If you want me to change it to use constants xml. Then let me know what can be ideal contant value for horizonal padding and vertical margin. Figma does not justify these values.

[conroy-ricketts]

+1 on what Shubham said about defining constant values in our dimens.xml files that match the Figma as reasonably close as possible

[Jignesh-dimagi]

@jaypanchal-13 I am confused here. Looking to your screenshots, only difference is that vertical padding for sign out view from top header. That is the requirement or something else? Also, agree with the point that changes has to be in xml file only and not in code.

[jaypanchal-13]

@jaypanchal-13 I am confused here. Looking to your screenshots, only difference is that vertical padding for sign out view from top header. That is the requirement or something else? Also, agree with the point that changes has to be in xml file only and not in code.

@Jignesh-dimagi yes with that there is change in horizontal padding

[Jignesh-dimagi]

@Jignesh-dimagi yes with that there is change in horizontal padding

Ok got it. So, this can be handled by having dp in xml file only?

[jaypanchal-13]

@Jignesh-dimagi yes with that there is change in horizontal padding

Ok got it. So, this can be handled by having dp in xml file only?

@Jignesh-dimagi For different display's it will not be visualised properly. So I added percentage approch. But will be changing it to use dimens.xml

[shubham1g5]

If you want me to change it to use constants xml. Then let me know what can be ideal contant value for horizonal padding and vertical margin. Figma does not justify these values

You can use the same % calculation to arrive at a rough estimation per smallest width classifier