Skip to content

Conversation

@FerGabPereira
Copy link
Contributor

@FerGabPereira FerGabPereira commented Aug 18, 2025

I created as a draft PR just to check if this PR meats the expectations from codeowner.

Changelog:

Adding in relative and standing settings a toogle to enabled/disabled the car number.

My only concern with this is that we lost the visual separator, because car number serve as a column that separates the position with the rest of the data.

Screenshots:

Standings:

Multi class race:
image

Single class race:
image

Relatives:

Multi class race:
image

Single race:

image

@tariknz tariknz marked this pull request as ready for review August 18, 2025 19:49
Copilot AI review requested due to automatic review settings August 18, 2025 19:49
Copy link
Contributor

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 adds a toggle feature to hide or show car numbers in both the standings and relative widgets. The author has noted a concern about losing the visual separator that car numbers provide between position and other data.

  • Added carNumber configuration option to both standings and relative widget settings
  • Made car number display conditional based on the toggle setting
  • Updated default configurations to include the new car number toggle

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
DriverInfoRow.tsx Made carNumber prop optional and conditionally renders the car number cell
Standings.tsx Added conditional logic to pass carNumber based on settings
Relative.tsx Added conditional logic to pass carNumber based on settings
types.ts Added carNumber enabled boolean to both widget settings interfaces
StandingsSettings.tsx Added toggle switch for car number visibility with migration logic
RelativeSettings.tsx Added toggle switch for car number visibility with migration logic
defaultDashboard.ts Added default carNumber enabled setting to dashboard configuration

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@tariknz
Copy link
Owner

tariknz commented Aug 18, 2025

Looks good to me. Just regarding your note about the visual seperater I'm happy if you want to play around with it but I think it looks fine to me honestly.

Just one small request can you include a demo/screenshot in the PR description so I can include it in the release notes. Other than that happy to approve and merge!

@FerGabPereira
Copy link
Contributor Author

Looks good to me. Just regarding your note about the visual seperater I'm happy if you want to play around with it but I think it looks fine to me honestly.

Just one small request can you include a demo/screenshot in the PR description so I can include it in the release notes. Other than that happy to approve and merge!

Done, i like more how i looks with the separator specially in multi class races

@tariknz
Copy link
Owner

tariknz commented Aug 19, 2025

Looks good to me. Just regarding your note about the visual seperater I'm happy if you want to play around with it but I think it looks fine to me honestly.
Just one small request can you include a demo/screenshot in the PR description so I can include it in the release notes. Other than that happy to approve and merge!

Done, i like more how i looks with the separator specially in multi class races

Can you just double check the relative overlay, standings seem to be fine but relative looks broken when car number is hidden:

image

@tariknz tariknz merged commit 0a45d16 into tariknz:main Aug 23, 2025
1 check passed
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