Skip to content

Conversation

Copy link

Copilot AI commented Jun 3, 2025

This PR removes support for stones as a weight unit from the Dropped cycling app, focusing only on imperial (pounds) and metric (kilograms) units as requested in the issue.

Changes Made

Core Model Updates

  • WeightUnit enum: Removed .stones = "st" case from the enum
  • Conversion logic: Removed stones conversion logic from the convert() function
  • Migration: Added automatic migration for existing users with stones to convert them to pounds

UI Simplification

  • OnboardingView: Removed the stones picker UI that appeared in imperial mode
  • SettingsView: Removed stones option from the weight unit picker
  • OnboardingViewModel: Removed showStonesOption property and related logic

Test Updates

  • UserDataTests: Removed stones conversion tests and added migration verification
  • OnboardingViewModelTests: Updated unit selection tests to focus on pounds ↔ kilograms conversion
  • New tests: Added verification that only 2 weight units exist (pounds and kilograms)

Migration Strategy

Existing users who had stones selected will be automatically migrated to pounds when their data is loaded. The weight value remains stored in kilograms internally for consistency, but the display unit preference is changed from "st" to "lb".

Before/After Comparison

Before: 3 weight units (pounds, kilograms, stones)

enum WeightUnit: String, CaseIterable {
    case pounds = "lb"
    case kilograms = "kg" 
    case stones = "st"  // ❌ Removed
}

After: 2 weight units (pounds, kilograms)

enum WeightUnit: String, CaseIterable {
    case pounds = "lb"
    case kilograms = "kg"
}

The UI now presents a clean imperial/metric toggle without the additional complexity of stones selection.

Fixes #11.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Remove stones as a unit Remove stones as a weight unit, keeping only pounds and kilograms Jun 3, 2025
Copilot AI requested a review from pierceboggan June 3, 2025 05:26
@pierceboggan pierceboggan marked this pull request as ready for review June 4, 2025 23:32
@pierceboggan pierceboggan requested a review from Copilot June 13, 2025 03:51
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes support for stones as a weight unit, streamlines the UI, and adds migration for existing stone-based users to pounds.

  • Removed .stones cases and conversion logic from models
  • Simplified onboarding and settings views to only handle pounds and kilograms
  • Introduced a migration path in UserDataManager and updated tests accordingly

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
Dropped/Models/UserData.swift Removed stones enum cases and conversion logic; added migration logic
Dropped/Views/SettingsView.swift Removed stones picker UI
Dropped/Views/OnboardingView.swift Removed stones option in onboarding
Dropped/ViewModels/OnboardingViewModel.swift Removed showStonesOption and related logic
DroppedTests/UserDataTests.swift Removed stones tests; added migration and allCases verification
DroppedTests/OnboardingViewModelTests.swift Updated tests to reflect pounds↔kilograms flows

if userData.weightUnit == "st" {
var migratedData = userData
migratedData.weightUnit = WeightUnit.pounds.rawValue
self.userData = migratedData
Copy link

Copilot AI Jun 13, 2025

Choose a reason for hiding this comment

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

The migration logic updates in-memory userData but does not persist these changes to storage. Consider calling saveUserData(migratedData) within this block to ensure the migration is saved.

Suggested change
self.userData = migratedData
saveUserData(migratedData)

Copilot uses AI. Check for mistakes.

func loadUserData() -> UserData {
// Migrate any existing stones users to pounds
if userData.weightUnit == "st" {
Copy link

Copilot AI Jun 13, 2025

Choose a reason for hiding this comment

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

[nitpick] Avoid using the hardcoded literal "st". Define a legacy constant or extension to represent the old stones unit, which improves readability and reduces magic strings.

Suggested change
if userData.weightUnit == "st" {
if userData.weightUnit == WeightUnit.stones.rawValue {

Copilot uses AI. Check for mistakes.
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.

Remove stones as a unit

2 participants