Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Free mode with basic ui #387

Merged
merged 3 commits into from
Sep 9, 2024
Merged

Add Free mode with basic ui #387

merged 3 commits into from
Sep 9, 2024

Conversation

morteako
Copy link
Collaborator

@morteako morteako commented Sep 8, 2024

Adds basic free mode support.
Just an MVP, but should be fine
Treats 0 as free mode, and then shows this in the UI.
Adds a helping text in the editor to help peopple

image image image

For later:

  • Have a better free mode option in the workout editor
  • Have free mode with watt target in the workout editor

@morteako morteako marked this pull request as ready for review September 9, 2024 06:07
@morteako morteako force-pushed the freemode branch 2 times, most recently from 93ab117 to 1866b3f Compare September 9, 2024 06:16
@morteako morteako changed the title Feature : free mode - with basic ui Add Free mode - with basic ui Sep 9, 2024
@sivertschou sivertschou changed the title Add Free mode - with basic ui Add free mode with basic ui Sep 9, 2024
Copy link
Owner

@sivertschou sivertschou left a comment

Choose a reason for hiding this comment

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

LGTMADTSC

apps/frontend/src/hooks/useSmartTrainerInterface.ts Outdated Show resolved Hide resolved
@@ -206,6 +206,8 @@ export const WorkoutEditor = ({
</InputGroup>
</FormControl>

<Text fontWeight={'light'}>Tip:Use 0 as watt target for Free Mode</Text>
Copy link
Owner

@sivertschou sivertschou Sep 9, 2024

Choose a reason for hiding this comment

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

Can call it free mode for now, but I can't really see Free mode_ being used as a term. Some mention Resistance mode, Standard mode and SIM mode – we should decide what to call this 🤠

Anyways, I have some minor suggestions for the text itself, and I also suggest moving the text beneath the Add part button.

      <Text fontSize="small">
        Tip: Use 0 as power target for <em>Free mode</em>
      </Text>

aka:

  • Use fontSize instead of fontWeight
  • We don't need {} around component String params
  • Add spacing after Tip:
  • Call it Power target instead of Watt target, since there is a column named Power above said field
  • Free mode instead of Free Mode – That's also what's been used the other places it is mentioned in this PR 🤓
  • Add emphasis around Free mode
image

@sivertschou sivertschou changed the title Add free mode with basic ui Add Free mode with basic ui Sep 9, 2024
@morteako morteako merged commit 7b714aa into main Sep 9, 2024
3 checks passed
@morteako morteako deleted the freemode branch September 9, 2024 20:14
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