Skip to content

Conversation

@SherylNyawira
Copy link
Collaborator

@SherylNyawira SherylNyawira commented Nov 5, 2025

Description

This PR improves the light theme visibility and contrast across MpesaKit components.
Previously, card backgrounds and text colors were too light, reducing readability and visual separation between UI elements.
#100
Key updates:

Adjusted --card-bg to a light neutral tone for better card visibility.

Set --text-primary to dark gray (#1a1a1a) for improved legibility on white surfaces.

Added subtle borders to buttons and feature cards for clearer definition.

Updated API status section background to a faint green tint for softer contrast.

Ensured code text and section labels maintain readable contrast.

Cleaned redundant CSS (moved inline styles to components where applicable).

This enhances the overall accessibility and user experience in light mode without affecting the dark theme.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires documentation update
  • Refactor (code structure improvements, no new functionality)
  • Tests (addition or improvement of tests)
  • Chore (changes to tooling, CI/CD, or metadata)

How Has This Been Tested?

Verified changes manually in both light and dark modes.

Checked visibility of cards, buttons, and API status section on different screen brightness levels.

Confirmed no layout or style regressions in dark mode.

Checklist

  • My code follows the project's coding style guidelines
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (if applicable)
  • My changes generate no new warnings or errors
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Screenshots (if applicable)

Additional Context

This fix ensures the MpesaKit documentation and component demos remain visually accessible and consistent with the global Docusaurus theme.

Summary by CodeRabbit

  • Style

    • Updated color tokens and UI styling across components with improved contrast.
    • Added borders and rounded corners to buttons, cards, and code sections.
    • Enhanced styling for better dark/light theme support.
  • Chores

    • Refactored HTTP client implementation and related tests.
    • Updated error handling mechanisms.

@coderabbitai
Copy link

coderabbitai bot commented Nov 5, 2025

Walkthrough

This PR migrates the HTTP client library from requests to httpx across the main codebase and test suites, introducing context manager patterns and updating error handling. Additionally, CSS styling is updated with new color tokens, borders, rounded corners, and dark/light mode adjustments.

Changes

Cohort / File(s) Summary
CSS Styling Updates
docs/src/components/MpesaKit.module.css
Removed global dark background tokens (--dark-bg), updated card and text colors (--card-bg to rgba(0,0,0,0.05), --text-primary to #1a1a1a). Added borders and rounded corners to buttons (1px solid, 50px radius) and feature cards (1px border). Updated code block colors for contrast, API status background to lighter green, and security visuals handling moved inline.
HTTP Client Library Migration
mpesakit/http_client/mpesa_http_client.py
Replaced requests library with httpx, introduced httpx.Client context manager with 10-second timeout. Updated error detection logic: POST uses status_code >= 400, GET uses is_success. Exception types changed to httpx.TimeoutException and httpx.RequestError. Error payloads extended to include status_code and raw_response where applicable.
Integration Test Updates
tests/integration/mpesa_express/test_stk_push_e2e.py
Replaced requests library with httpx, updated imports and converted direct requests.post/get calls to use httpx.Client context manager for network interactions while preserving control flow and timing logic.
Unit Test Updates
tests/unit/http_client/test_mpesa_http_client.py
Updated test mocks to patch httpx.Client methods instead of requests methods, replaced exception types to httpx equivalents (httpx.RequestError, httpx.TimeoutException, httpx.ConnectError), and updated response mocks to use status_code instead of ok attribute.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • HTTP client error handling: Verify that the error detection logic transitions (status_code >= 400 vs is_success) correctly preserve the intended behavior and that error payloads are properly constructed with new fields.
  • Timeout handling: Confirm that httpx.TimeoutException is correctly caught and handled across all code paths.
  • Test mock correctness: Ensure that httpx.Client patches are correctly targeting the right mock targets and that exception side-effects align with actual httpx library behavior.
  • Context manager lifecycle: Verify that httpx.Client context managers are properly used and disposed in all code paths, especially in error scenarios.

Suggested reviewers

  • RafaelJohn9

Poem

🐰 From requests old to httpx new,
With context managers and timeouts true,
Error handling refined, styling takes flight,
Colors and borders make everything bright!

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Fix light mode' is vague and does not clearly convey the specific nature of the changes made to the CSS styling and theme adjustments. Consider a more descriptive title such as 'Improve light mode visibility and contrast in MpesaKit components' to better reflect the scope of CSS adjustments.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The PR description is comprehensive, follows the template structure, includes all key sections (Description, Type of Change, Testing, Checklist), provides clear context about the light theme improvements, and addresses the motivation behind the changes.
✨ 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 fixLightMode

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

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (6)
docs/src/components/MpesaKit.module.css (1)

857-857: Critical: Undefined CSS variable reference will break the gradient.

Line 857 references var(--dark-bg), but this variable was removed from :root per the PR changes. The linear gradient will be invalid and .ctaSection will lose its intended background styling.

Apply this diff to fix the gradient:

-.ctaSection {
+.ctaSection {
-  background: linear-gradient(135deg, var(--dark-bg) 0%, rgba(0, 209, 58, 0.05) 100%);
+  background: linear-gradient(135deg, rgba(0, 0, 0, 0.02) 0%, rgba(0, 209, 58, 0.05) 100%);

Adjust the rgba(0, 0, 0, 0.02) value if needed to align with your light mode aesthetic.

tests/unit/http_client/test_mpesa_http_client.py (2)

112-114: Add is_success attribute to mock response.

The implementation at mpesakit/http_client/mpesa_http_client.py line 128 checks response.is_success, but the mock response doesn't set this attribute. This may cause the GET success test to fail.

Apply this diff:

         mock_response = Mock()
         mock_response.status_code = 200
+        mock_response.is_success = True
         mock_response.json.return_value = {"foo": "bar"}

138-142: Add is_success attribute to mock response.

The implementation at mpesakit/http_client/mpesa_http_client.py line 128 checks response.is_success, but the mock response doesn't set this attribute, causing the test to fail per the pipeline failure log.

Apply this diff:

         mock_response = Mock()
         mock_response.status_code = 500
+        mock_response.is_success = False
         mock_response.json.side_effect = ValueError()
mpesakit/http_client/mpesa_http_client.py (3)

1-157: Critical: PR description does not match the actual code changes.

The PR is titled "Fix light mode" and the description discusses CSS changes for improving visibility and contrast in light theme. However, the actual changes in this file (and others) migrate the HTTP client from requests to httpx. This is a significant inconsistency that needs to be addressed.

Please update the PR title and description to accurately reflect the HTTP client migration changes, or verify that the correct files were included in this PR.


78-94: Add separate handling for ConnectError.

The tests at tests/unit/http_client/test_mpesa_http_client.py lines 97-106 expect error_code "CONNECTION_ERROR" when httpx.ConnectError is raised, but the current implementation catches all httpx.RequestError exceptions and returns "REQUEST_FAILED". httpx.ConnectError is a specific network error type for failed connections.

Apply this diff to add separate handling:

         except httpx.TimeoutException:
             raise MpesaApiException(
                 MpesaError(
                     error_code="REQUEST_TIMEOUT",
                     error_message="Request to Mpesa timed out.",
                     status_code=None,
                 )
             )
+        except httpx.ConnectError as e:
+            raise MpesaApiException(
+                MpesaError(
+                    error_code="CONNECTION_ERROR",
+                    error_message=f"Failed to connect: {str(e)}",
+                    status_code=None,
+                    raw_response=None,
+                )
+            )
         except httpx.RequestError as e:
             raise MpesaApiException(
                 MpesaError(
                     error_code="REQUEST_FAILED",
                     error_message=f"HTTP request failed: {str(e)}",
                     status_code=None,
                     raw_response=None,
                 )
             )

141-157: Add separate handling for ConnectError in GET method.

Similar to the POST method, the GET method needs separate handling for httpx.ConnectError to match test expectations at lines 172-180. The tests expect error_code "CONNECTION_ERROR" for connection failures.

Apply this diff to add separate handling:

         except httpx.TimeoutException:
             raise MpesaApiException(
                 MpesaError(
                     error_code="REQUEST_TIMEOUT",
                     error_message="Request to Mpesa timed out.",
                     status_code=None,
                 )
             )
+        except httpx.ConnectError as e:
+            raise MpesaApiException(
+                MpesaError(
+                    error_code="CONNECTION_ERROR",
+                    error_message=f"Failed to connect: {str(e)}",
+                    status_code=None,
+                    raw_response=None,
+                )
+            )
         except httpx.RequestError as e:
             raise MpesaApiException(
                 MpesaError(
                     error_code="REQUEST_FAILED",
                     error_message=f"HTTP request failed: {str(e)}",
                     status_code=None,
                     raw_response=None,
                 )
             )
🧹 Nitpick comments (3)
docs/src/components/MpesaKit.module.css (1)

144-146: Remove duplicate border-radius declaration.

Lines 145 and 146 both define border-radius: 50px; on the same .btn rule. The duplication is redundant.

Apply this diff to remove the duplicate:

  .btn {
    padding: 0.875rem 1.5rem;
    border: 1px solid rgba(0, 0, 0, 0.1);
-   border-radius: 50px;
    border-radius: 50px;
    font-weight: 600;
tests/integration/mpesa_express/test_stk_push_e2e.py (2)

85-86: Consider specifying timeout for consistency.

The httpx.Client() context manager should specify a timeout to match the pattern used in mpesakit/http_client/mpesa_http_client.py (line 57, 120) which uses timeout=10.

Apply this diff:

-    with httpx.Client() as client:
-        client.post(f"{callback_base_url}/clear")
+    with httpx.Client(timeout=10) as client:
+        client.post(f"{callback_base_url}/clear")

115-127: Consider specifying timeout for consistency.

The httpx.Client() context manager should specify a timeout to match the pattern used in mpesakit/http_client/mpesa_http_client.py. Note that line 118 already specifies timeout=45 in the individual get() call, but for consistency, it's better to set it at the client level.

Apply this diff:

-    with httpx.Client() as client:
+    with httpx.Client(timeout=45) as client:
         for _ in range(30):
             time.sleep(1)
-            r = client.get(f"{callback_base_url}/latest", timeout=45)
+            r = client.get(f"{callback_base_url}/latest")
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ffd51ae and a756909.

📒 Files selected for processing (4)
  • docs/src/components/MpesaKit.module.css (5 hunks)
  • mpesakit/http_client/mpesa_http_client.py (5 hunks)
  • tests/integration/mpesa_express/test_stk_push_e2e.py (3 hunks)
  • tests/unit/http_client/test_mpesa_http_client.py (13 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/integration/mpesa_express/test_stk_push_e2e.py (2)
mpesakit/http_client/mpesa_http_client.py (2)
  • post (39-94)
  • get (96-157)
mpesakit/mpesa_express/schemas.py (1)
  • StkPushSimulateCallback (351-452)
mpesakit/http_client/mpesa_http_client.py (1)
mpesakit/errors.py (3)
  • MpesaApiException (33-49)
  • MpesaError (11-30)
  • error_code (42-44)
🪛 GitHub Actions: Run Unit Tests
tests/unit/http_client/test_mpesa_http_client.py

[error] 93-93: Test post_timeout failed: Mock side_effect raises httpx.TimeoutException which triggers TypeError: RequestError.init() missing 'message'.


[error] 104-104: Test post_connection_error failed: Mock side_effect raises httpx.ConnectError leading to TypeError: RequestError.init() missing 'message'.


[error] 129-129: Test_get_http_error failed: MpesaApiException not raised as expected when HTTP error response is returned.


[error] 144-144: Test_get_json_decode_error failed: MpesaApiException not raised when JSON decode raises ValueError.


[error] 168-168: Test_get_timeout failed: Mock side_effect raises httpx.TimeoutException which triggers TypeError: RequestError.init() missing 'message'.


[error] 179-179: Test_get_connection_error failed: Mock side_effect raises httpx.ConnectError leading to TypeError: RequestError.init() missing 'message'.

🔇 Additional comments (5)
docs/src/components/MpesaKit.module.css (3)

261-261: Verify color contrast for code text on dark gradient background.

The .codeContent text is set to #f8f8f8 (very light gray) against the .codeDemo dark gradient background. While this generally provides adequate contrast, confirm that the combination meets WCAG accessibility standards when viewed at various brightness levels, as mentioned in the PR checklist.

Verify the contrast ratio between #f8f8f8 (foreground) and the gradient in .codeDemo (background) meets WCAG AA standards (4.5:1 minimum for text).


7-8: Light mode color tokens are well-positioned.

The updates to --card-bg: rgba(0, 0, 0, 0.05) and --text-primary: #1a1a1a establish a clear light-mode foundation with improved contrast. These changes align well with the PR objective of improving visibility.


746-747: API Status section styling supports light mode readability.

The background adjustment to rgba(0, 209, 58, 0.02) with the explicit color: var(--text-primary) ensures text remains readable in light mode while maintaining the subtle green tint for visual context.

tests/unit/http_client/test_mpesa_http_client.py (2)

176-176: Fix httpx.ConnectError instantiation.

httpx.ConnectError requires a message parameter in its constructor. The mock side_effect is missing this required parameter, causing the test to fail.

Apply this diff:

-        side_effect=httpx.ConnectError,
+        side_effect=httpx.ConnectError("Connection failed"),

Likely an incorrect or invalid review comment.


90-90: Fix httpx.TimeoutException instantiation.

httpx.TimeoutException requires a message parameter in its constructor. The mock side_effect is missing this required parameter, causing the test to fail.

Apply this diff:

-        side_effect=httpx.TimeoutException,
+        side_effect=httpx.TimeoutException("Connection timeout"),

Likely an incorrect or invalid review comment.

Comment on lines +617 to 618
border: 1px solid rgba(0, 0, 0, 0.1);
border: 1px solid rgba(255, 255, 255, 0.1);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Conflicting border definitions on .featureCard.

Lines 617 and 618 both set border, but with different colors: rgba(0, 0, 0, 0.1) (dark) and rgba(255, 255, 255, 0.1) (light). The second declaration will override the first, making the first line ineffective.

Clarify the intended border style and apply a single border definition:

  .featureCard {
    background: var(--card-bg);
    border-radius: 20px;
    padding: 2.5rem;
-   border: 1px solid rgba(0, 0, 0, 0.1);
    border: 1px solid rgba(255, 255, 255, 0.1);
    transition: all 0.3s ease;

Alternatively, if the intent is to use a dark border for light mode, remove line 618 and keep line 617.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
border: 1px solid rgba(0, 0, 0, 0.1);
border: 1px solid rgba(255, 255, 255, 0.1);
border: 1px solid rgba(255, 255, 255, 0.1);
🧰 Tools
🪛 Biome (2.1.2)

[error] 618-618: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.

border is already defined here.

Remove or rename the duplicate property to ensure consistent styling.

(lint/suspicious/noDuplicateProperties)

🤖 Prompt for AI Agents
In docs/src/components/MpesaKit.module.css around lines 617-618 there are two
conflicting border declarations for .featureCard (rgba(0,0,0,0.1) and
rgba(255,255,255,0.1)) so remove the unintended one and keep the intended color;
if you need different borders for light/dark themes instead of duplicate lines,
replace them with a single base border and add a prefers-color-scheme media
query (or use a CSS variable) to override the color for dark mode.


from typing import Dict, Any, Optional
import requests
import httpx
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify httpx is declared as a dependency.

The code now imports and uses httpx, but there's no indication that this dependency has been added to the project's dependency manifest (e.g., requirements.txt, setup.py, pyproject.toml). This will cause import errors in production.

Run the following script to check if httpx is declared as a dependency:


🏁 Script executed:

#!/bin/bash
# Check if httpx is declared in common dependency files

echo "=== Checking for httpx in dependency files ==="

fd -t f 'requirements.*\.txt$|setup\.py$|pyproject\.toml$|Pipfile$' --exec echo "File: {}" \; --exec grep -n "httpx" {} \; 2>/dev/null || echo "No httpx dependency found"

Length of output: 95


I need to inspect the actual contents of pyproject.toml to verify the current dependencies.


🏁 Script executed:

#!/bin/bash
# View the contents of pyproject.toml to see current dependencies
cat ./pyproject.toml

Length of output: 2651


Add httpx to project dependencies in pyproject.toml.

The code imports httpx, but it is not declared in the dependencies list. This will cause import errors when the package is installed. Add httpx to the dependencies section in pyproject.toml:

dependencies = [ 
  "pydantic >=2.10.6,<3.0.0",
  "pydantic[email] >=2.10.6,<3.0.0",
  "requests >=2.32.3,<3.0.0",
  "typing_extensions >= 4.12.2,<5.0.0",
  "cryptography >=41.0.7",
  "httpx",  # Add version constraint as appropriate
]
🤖 Prompt for AI Agents
In mpesakit/http_client/mpesa_http_client.py at line 7 the module imports httpx
but the package doesn't declare httpx in pyproject.toml; open pyproject.toml,
locate the [project] / dependencies list and add an appropriate
version-constrained entry for "httpx" (for example "httpx >=0.24.0,<1.0.0" or
whatever your compatibility policy requires), save and run
poetry/pipenv/installer to update lockfiles so the dependency is installed
during packaging and CI.

"mpesakit.http_client.mpesa_http_client.requests.post",
side_effect=requests.ConnectionError,
"mpesakit.http_client.mpesa_http_client.httpx.Client.post",
side_effect=httpx.ConnectError,
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix httpx.ConnectError instantiation.

httpx.ConnectError requires a message parameter in its constructor. The mock side_effect is missing this required parameter, causing the test to fail.

Apply this diff:

-        side_effect=httpx.ConnectError,
+        side_effect=httpx.ConnectError("Connection failed"),
🤖 Prompt for AI Agents
In tests/unit/http_client/test_mpesa_http_client.py around line 101 the mock
sets side_effect=httpx.ConnectError but httpx.ConnectError requires a message
parameter; replace the side_effect with a properly instantiated exception (for
example side_effect=httpx.ConnectError("connection failed") or a callable that
raises httpx.ConnectError with a descriptive message) so the mock raises a valid
ConnectError instance.

Comment on lines 124 to 127
mock_response = Mock()
mock_response.ok = False
mock_response.status_code = 404
mock_response.json.return_value = {"errorMessage": "Not Found"}
mock_get.return_value = mock_response
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Add is_success attribute to mock response.

The implementation at mpesakit/http_client/mpesa_http_client.py line 128 checks response.is_success, but the mock response doesn't set this attribute, causing the test to fail per the pipeline failure log.

Apply this diff:

         mock_response = Mock()
         mock_response.status_code = 404
+        mock_response.is_success = False
         mock_response.json.return_value = {"errorMessage": "Not Found"}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
mock_response = Mock()
mock_response.ok = False
mock_response.status_code = 404
mock_response.json.return_value = {"errorMessage": "Not Found"}
mock_get.return_value = mock_response
mock_response = Mock()
mock_response.status_code = 404
mock_response.is_success = False
mock_response.json.return_value = {"errorMessage": "Not Found"}
mock_get.return_value = mock_response
🤖 Prompt for AI Agents
In tests/unit/http_client/test_mpesa_http_client.py around lines 124 to 127, the
mock response lacks the is_success attribute expected by
mpesakit/http_client/mpesa_http_client.py; set mock_response.is_success = False
(for this 404 case) so the test mimics the real response shape; for completeness
ensure other mocked responses in the file set is_success=True/False consistently
to match expected status branches.

"mpesakit.http_client.mpesa_http_client.requests.get",
side_effect=requests.Timeout,
"mpesakit.http_client.mpesa_http_client.httpx.Client.get",
side_effect=httpx.TimeoutException,
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix httpx.TimeoutException instantiation.

httpx.TimeoutException requires a message parameter in its constructor. The mock side_effect is missing this required parameter, causing the test to fail.

Apply this diff:

-        side_effect=httpx.TimeoutException,
+        side_effect=httpx.TimeoutException("Connection timeout"),
🤖 Prompt for AI Agents
In tests/unit/http_client/test_mpesa_http_client.py around line 165, the mock
side_effect uses httpx.TimeoutException without the required message argument;
update the mock to raise an instantiated exception with a message (e.g., use
side_effect=httpx.TimeoutException("timeout") or a lambda that raises
httpx.TimeoutException("timeout")) so the constructor receives the required
`message` parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants