Skip to content

Conversation

@tamaskozmer
Copy link
Contributor

@tamaskozmer tamaskozmer commented Jan 5, 2026

Test plan:

  1. Set a course's home page to Recent Activity (Notifications)
  2. Put the device in offline mode (disable network)
  3. Navigate to the Course Browser for that course
  4. Verify that the Home button is disabled (grayed out with 50% opacity)
  5. Verify that clicking the Home button does nothing
  6. Re-enable network
  7. Verify that the Home button becomes enabled again

refs: MBL-19630
affects: Student
release note: Fixed Home button to be disabled when offline and home page is set to Recent Activity

Checklist

  • Follow-up e2e test ticket created or not needed
  • Tested in dark mode
  • Tested in light mode
  • Test in landscape mode and/or tablet
  • A11y checked
  • Approve from product

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Review Summary

This PR adds offline support by disabling the Recent Activity home tab when the device is offline. The implementation is generally solid with good separation of concerns.

Positive Aspects

✅ Good use of dependency injection - passing isOnline through the adapter constructor
✅ Proper use of safe casting with as? for type safety
✅ Visual feedback with alpha transparency for disabled state
✅ Correctly prevents clicks by both disabling the view and removing the click listener

Issues Found

  • Accessibility: The disabled state only uses visual alpha transparency. Screen reader users won't be informed that the item is unavailable offline (see inline comment on line 89)
  • Code style: The click listener logic could be simplified to be more concise (see inline comment on line 90)

Additional Considerations

Testing recommendations:

  • Verify behavior when canvasContext is not a Course type
  • Test with TalkBack/screen readers enabled to ensure accessibility
  • Test switching between online/offline states to ensure the UI updates correctly
  • Verify that the alpha and enabled state reset properly when coming back online

Performance:
The implementation looks efficient - no concerns with the current approach.

Security:
No security concerns identified in this change.

Overall, this is a clean implementation that just needs some accessibility improvements before merging.

@github-actions
Copy link

github-actions bot commented Jan 5, 2026

📊 Code Coverage Report

✅ Student

  • PR Coverage: 43.55%
  • Master Coverage: 43.55%
  • Delta: +0.00%

✅ Teacher

  • PR Coverage: 25.61%
  • Master Coverage: 25.61%
  • Delta: +0.00%

✅ Pandautils

  • PR Coverage: 22.92%
  • Master Coverage: 22.92%
  • Delta: +0.00%

📈 Overall Average

  • PR Coverage: 30.69%
  • Master Coverage: 30.69%
  • Delta: +0.00%

@github-actions
Copy link

github-actions bot commented Jan 5, 2026

🧪 Unit Test Results

✅ 📱 Student App

  • Tests: 1241 total, 0 failed, 0 skipped
  • Duration: 0.000s
  • Success Rate: 100%

✅ 🌅 Horizon

  • Tests: 449 total, 0 failed, 0 skipped
  • Duration: 33.354s
  • Success Rate: 100%

✅ 📦 Submodules

  • Tests: 2515 total, 0 failed, 0 skipped
  • Duration: 46.874s
  • Success Rate: 100%

📊 Summary

  • Total Tests: 4205
  • Failed: 0
  • Skipped: 0
  • Status: ✅ All tests passed!

Last updated: Mon, 05 Jan 2026 11:59:42 GMT

@github-actions
Copy link

github-actions bot commented Jan 5, 2026

Student Install Page

Copy link
Contributor

@kdeakinstructure kdeakinstructure left a comment

Choose a reason for hiding this comment

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

QA 👍

@tamaskozmer tamaskozmer merged commit 83ede14 into master Jan 6, 2026
31 checks passed
@tamaskozmer tamaskozmer deleted the MBL-19630-disable-home-recent-activity-in-offline branch January 6, 2026 13:40
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.

4 participants