-
-
Notifications
You must be signed in to change notification settings - Fork 58
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
feature/race-control-chime #188
base: develop
Are you sure you want to change the base?
Conversation
uses settingsStore to store the setting. compares time of message to local time to determine if message is older than when the user went on the website. gives every message a id so that it isnt played twice.
@Ellodev is attempting to deploy a commit to the f1-dash Team on Vercel. A member of the Team first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the PR 🙏🏻
There's a couple things that u might want to look into.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay so the code is fine for me like this. If u want to you cold add a volume slider in the settings so the users can adjust the volume to their liking. I think that would be a neat addition and fairly straight forward to implement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for adding the volume slider, I am starting to like it. Just a few minor adjustments and we are ready to
setValue: (value: number) => void; | ||
}; | ||
|
||
export default function Input({ value, setValue }: Props) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this Input has custom code for numbers, maybe it would be a good idea to either call it numeric input or move the numeric logic to where its used, what do you think is best?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking we could maybe call it something like PercentageInput since its defined as a Maximum of 100, and used for Percentages. Do you agree or would you rather that I move the Number logic somewhere else to make it more reusable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we go for Input for now and move the number logic to the settings page where we update the store
… feature/race-control-chime
uses settingsStore to store the setting.
compares time of message to local time to determine if message is older than when the user went on the website. gives every message a id so that it isnt played twice.