Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .chart-refactor-branch
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Chart Popover Positioning Refactoring

This branch contains the chart popover positioning refactoring.
See CHART_TESTING_GUIDE.md for testing instructions.
163 changes: 163 additions & 0 deletions CREATE_SECOND_PR.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
# How to Create the Second PR (Chart Popover Positioning)

I've split your refactoring into two separate branches as requested:

1. **Settings Hint Management** - Already in `copilot/refactor-duplicated-code` ✅
2. **Chart Popover Positioning** - Ready in local branch `copilot/refactor-chart-popover-positioning` ⏳

## Option 1: Push the Branch (Recommended)

The second branch exists locally but needs to be pushed to GitHub:

```bash
# Make sure you're in the Trio repository
cd /path/to/Trio

# Fetch the latest changes
git fetch origin

# Check out the chart popover branch
git checkout copilot/refactor-chart-popover-positioning

# Verify it has the right commits (should show 2 commits)
git log --oneline -5

# Push to GitHub
git push -u origin copilot/refactor-chart-popover-positioning
```

Then create a PR on GitHub from the `copilot/refactor-chart-popover-positioning` branch.

## Option 2: Recreate the Branch

If the local branch doesn't exist, you can recreate it:

```bash
# Start from the base commit
git checkout 99e4a0d

# Create the new branch
git checkout -b copilot/refactor-chart-popover-positioning

# Cherry-pick the chart refactoring commit
git cherry-pick dc20dcd

# Add the test files
# (Copy the files from the guide below)

# Commit and push
git add TrioTests/StatChartUtilsTests.swift CHART_TESTING_GUIDE.md
git commit -m "Add tests and testing guide for chart popover positioning refactoring"
git push -u origin copilot/refactor-chart-popover-positioning
```

## What's in the Chart Popover Branch

### Commits:
1. `dc20dcd` - Refactor duplicate xOffset chart popover calculation into StatChartUtils
2. `7da293e` - Add tests and testing guide for chart popover positioning refactoring

### Files Changed:
- `Trio/Sources/Modules/Stat/View/StatChartUtils.swift` (added method)
- `Trio/Sources/Modules/Stat/View/ViewElements/Meal/MealStatsView.swift` (refactored)
- `Trio/Sources/Modules/Stat/View/ViewElements/Insulin/TotalDailyDoseChart.swift` (refactored)
- `Trio/Sources/Modules/Stat/View/ViewElements/Insulin/BolusStatsView.swift` (refactored)
- `TrioTests/StatChartUtilsTests.swift` (new test file)
- `CHART_TESTING_GUIDE.md` (new manual testing guide)

### Testing:
- 10 automated unit tests
- Comprehensive manual testing guide for 3 chart views

## Current Status

✅ **PR 1: Settings Hint Management**
- Branch: `copilot/refactor-duplicated-code`
- Status: Pushed and ready
- Files: 15 settings views refactored
- Tests: 12 unit tests
- Guide: `SETTINGS_TESTING_GUIDE.md`

⏳ **PR 2: Chart Popover Positioning**
- Branch: `copilot/refactor-chart-popover-positioning` (local only)
- Status: Needs to be pushed
- Files: 3 chart views refactored
- Tests: 10 unit tests
- Guide: `CHART_TESTING_GUIDE.md`

## PR Descriptions

When creating the PRs on GitHub, you can use these descriptions:

### PR 1: Settings Hint Management
See the updated description in the current PR.

### PR 2: Chart Popover Positioning
```markdown
## Refactor Chart Popover Positioning

This PR eliminates code duplication across 3 chart views by extracting duplicate popover positioning logic into a shared utility function.

### Changes

Extracted identical 27-line `xOffset()` function to `StatChartUtils.calculatePopoverXOffset()`:

**Before (per chart):**
```swift
private func xOffset() -> CGFloat {
let domainDuration = domain.end.timeIntervalSince(domain.start)
guard domainDuration > 0, chartWidth > 0 else { return 0 }

let popoverWidth = popoverSize.width

// Convert dates to pixel'd x-condition
let dateFraction = selectedDate.timeIntervalSince(domain.start) / domainDuration
let x_selected = dateFraction * chartWidth

// ... 18 more lines calculating offset

return offset
}
```

**After (per chart):**
```swift
private func xOffset() -> CGFloat {
StatChartUtils.calculatePopoverXOffset(
domain: domain,
selectedDate: selectedDate,
chartWidth: chartWidth,
popoverWidth: popoverSize.width
)
}
```

### Testing

**Automated Tests:**
- `TrioTests/StatChartUtilsTests.swift` - 10 unit tests

**Manual Testing Guide:**
- `CHART_TESTING_GUIDE.md` - Detailed procedures for 3 chart views

### Impact

- **4 files modified**
- **Net reduction**: 17 lines

### Benefits

1. **Maintainability**: Logic centralized in one place
2. **Consistency**: All charts use the same algorithm
3. **Bug Fixes**: Fixes now apply to all charts automatically
4. **Testability**: Comprehensive test coverage
```

## Need Help?

If you encounter any issues:
1. Check that you have the latest commits fetched
2. Verify you're in the Trio repository directory
3. Make sure you have push permissions to the repository

You can also just merge the changes manually or let me know if you need a different approach!
218 changes: 218 additions & 0 deletions MANUAL_TESTING_GUIDE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,218 @@
# Manual Testing Guide for Refactored Code

This guide provides step-by-step instructions for manually testing the refactored code changes in the Trio app.

## Overview

Two main areas have been refactored:
1. **Settings Hint Management** - Affects 15 settings views
2. **Chart Popover Positioning** - Affects 3 chart views

## Automated Tests

Run the automated test suites first:

```bash
# In Xcode, run the test target
# Or use command line:
xcodebuild test -workspace Trio.xcworkspace -scheme Trio -destination 'platform=iOS Simulator,name=iPhone 15'
```

The following test files have been added:
- `TrioTests/SettingsHintManagerTests.swift` - Tests for SettingsHintManager
- `TrioTests/StatChartUtilsTests.swift` - Tests for chart popover positioning

## Manual Testing

### Part 1: Settings Hint Display (15 Views)

Test the hint display functionality in all affected settings views to ensure the refactored code works correctly.

#### Settings Views to Test:

1. **SMB Settings** (`Settings` → `SMB Settings`)
2. **Autosens Settings** (`Settings` → `Autosens Settings`)
3. **Autotune Config** (`Settings` → `Autotune Config`)
4. **CGM Settings** (`Settings` → `CGM Settings`)
5. **Dynamic Settings** (`Settings` → `Dynamic Settings`)
6. **Units & Limits Settings** (`Settings` → `General Settings` → `Units & Limits`)
7. **Meal Settings** (`Settings` → `Meal Settings`)
8. **Nightscout Config** (`Settings` → `Nightscout Config`)
9. **Nightscout Fetch** (within Nightscout Config)
10. **Nightscout Upload** (within Nightscout Config)
11. **Settings Root** (main settings page)
12. **Notifications View** (`Settings` → `Notifications`)
13. **Tidepool Start** (`Settings` → `Tidepool`)
14. **Shortcuts Config** (`Settings` → `Shortcuts`)
15. **Target Behavior** (`Settings` → `Target Behavior`)

#### Test Steps for Each Settings View:

1. **Navigate to the settings view**
- Open the Trio app
- Navigate to the specific settings page

2. **Test hint icon interaction**
- Look for the information icon (ℹ️) next to setting labels
- Tap the info icon
- **Expected**: A sheet should appear with detailed help information

3. **Verify hint content**
- Read the hint text in the sheet
- **Expected**: The hint label should match the setting name
- **Expected**: The hint content should be relevant to that setting

4. **Test hint dismissal**
- Tap the "Got it!" button or swipe down to dismiss
- **Expected**: The sheet should close smoothly

5. **Test multiple hints in same view**
- If the view has multiple settings with hints, test several
- **Expected**: Each hint should display its own specific content
- **Expected**: No content from previous hints should leak into new ones

6. **Test hint detent (sheet size)**
- When the sheet appears, try to drag it
- **Expected**: Sheet should support standard iOS sheet behaviors
- **Expected**: Sheet should not glitch or flicker

### Part 2: Chart Popover Positioning (3 Views)

Test the popover positioning in chart views to ensure they stay within bounds.

#### Chart Views to Test:

1. **Meal Stats Chart** (`Statistics` → `Meal` tab)
2. **Total Daily Dose Chart** (`Statistics` → `Insulin` tab)
3. **Bolus Stats Chart** (`Statistics` → `Insulin` tab)

#### Test Steps for Each Chart View:

1. **Navigate to the chart**
- Open the Trio app
- Go to the Statistics section
- Select the appropriate tab (Meal or Insulin)

2. **Test center data point selection**
- Tap on a data point in the middle of the chart
- **Expected**: A popover should appear above the data point
- **Expected**: The popover should be centered over the data point

3. **Test left edge data point**
- Tap on a data point near the left edge of the chart
- **Expected**: The popover should appear but shift right to stay within bounds
- **Expected**: The popover should not be cut off on the left side

4. **Test right edge data point**
- Tap on a data point near the right edge of the chart
- **Expected**: The popover should appear but shift left to stay within bounds
- **Expected**: The popover should not be cut off on the right side

5. **Test popover content**
- Verify that the popover displays correct data for the selected point
- **Expected**: Data values match the selected time/date

6. **Test with different time ranges**
- Change the time interval (day/week/month/total)
- Repeat tests 2-4 with different time ranges
- **Expected**: Popover positioning works correctly in all time ranges

7. **Test on different device sizes**
- Test on iPhone SE (small screen)
- Test on iPhone Pro Max (large screen)
- Test on iPad (if supported)
- **Expected**: Popover positioning adapts correctly to screen size

### Part 3: Regression Testing

Ensure that the refactoring hasn't broken any existing functionality.

#### General Functionality Tests:

1. **Settings persistence**
- Change a setting value
- Close and reopen the app
- **Expected**: Setting value is preserved

2. **Settings validation**
- Try to enter invalid values (if applicable)
- **Expected**: Validation works as before

3. **Chart interactions**
- Scroll through chart data
- Zoom (if applicable)
- Switch between different metrics
- **Expected**: All chart interactions work smoothly

4. **Performance**
- Navigate quickly between settings views
- Rapidly tap on different chart data points
- **Expected**: No lag or performance degradation
- **Expected**: No memory leaks

## Checklist

Use this checklist to track your testing progress:

### Settings Hint Display
- [ ] SMB Settings
- [ ] Autosens Settings
- [ ] Autotune Config
- [ ] CGM Settings
- [ ] Dynamic Settings
- [ ] Units & Limits Settings
- [ ] Meal Settings
- [ ] Nightscout Config
- [ ] Nightscout Fetch
- [ ] Nightscout Upload
- [ ] Settings Root
- [ ] Notifications View
- [ ] Tidepool Start
- [ ] Shortcuts Config
- [ ] Target Behavior

### Chart Popover Positioning
- [ ] Meal Stats Chart - center point
- [ ] Meal Stats Chart - left edge
- [ ] Meal Stats Chart - right edge
- [ ] Total Daily Dose Chart - center point
- [ ] Total Daily Dose Chart - left edge
- [ ] Total Daily Dose Chart - right edge
- [ ] Bolus Stats Chart - center point
- [ ] Bolus Stats Chart - left edge
- [ ] Bolus Stats Chart - right edge

### Regression Testing
- [ ] Settings persistence works
- [ ] Settings validation works
- [ ] Chart scrolling works
- [ ] No performance issues
- [ ] Tested on multiple device sizes

## Reporting Issues

If you find any issues during testing:

1. Note the specific view/chart where the issue occurs
2. Describe the steps to reproduce
3. Include screenshots or screen recordings if possible
4. Note the device model and iOS version
5. Report on the GitHub PR thread

## Additional Notes

### What Changed Internally

**Settings Hint Management:**
- Before: Each settings view managed its own hint state with 6 separate @State variables
- After: All hint state is now managed by a single `SettingsHintManager` object
- Impact: No visible change to end users, but more maintainable code

**Chart Popover Positioning:**
- Before: Each chart had its own copy of the `xOffset()` calculation function
- After: All charts use a shared `StatChartUtils.calculatePopoverXOffset()` function
- Impact: No visible change to end users, but fixes can now be applied once for all charts

### Expected Behavior

The refactoring should be **completely transparent** to users. If you notice ANY difference in behavior compared to the previous version, that's a bug that should be reported.
Loading