Skip to content

Change limit reductions labels #847

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 20 commits into from
Sep 12, 2025
Merged

Change limit reductions labels #847

merged 20 commits into from
Sep 12, 2025

Conversation

carojeandat
Copy link
Contributor

  • Invert labels and tooltips in the limit reductions table
  • Add tooltips on voltage ranges
  • Remove excess English capitalization in the limit reductions table of the safety analysis settings

antoinebhs
antoinebhs reviewed Sep 10, 2025
View reviewed changes
Copy link
Contributor

@antoinebhs antoinebhs left a comment

Choose a reason for hiding this comment

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

In addition to Sylvain's comments, one remark on translations

souissimai
souissimai reviewed Sep 11, 2025
View reviewed changes
src/components/parameters/common/limitreductions/limit-reductions-table-form.tsx Outdated
(limit: ITemporaryLimitReduction) => {
const lowBound = Math.trunc(limit.limitDuration.lowBound / 60);
const highBound = Math.trunc(limit.limitDuration.highBound / 60);
const highBound = Math.trunc(limit.limitDuration.lowBound / 60);
Copy link
Contributor

Choose a reason for hiding this comment

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

revert

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's on purpose. The higher the voltage, the shorter the time limit, so the high and low interval values are inverted

sBouzols
sBouzols approved these changes Sep 11, 2025
View reviewed changes
Copy link
Contributor

@sBouzols sBouzols left a comment

Choose a reason for hiding this comment

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

Code Review OK (two readOnly sonar issues to fix easily)

antoinebhs
antoinebhs approved these changes Sep 11, 2025
View reviewed changes
Copy link
Contributor

@antoinebhs antoinebhs left a comment

Choose a reason for hiding this comment

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

Code OK

antoinebhs
antoinebhs requested changes Sep 11, 2025
View reviewed changes
Copy link
Contributor

@antoinebhs antoinebhs left a comment •
edited
Loading

Choose a reason for hiding this comment

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

There is a scroll bar now, can we avoid that? Only in english actually because it's on two lines.

@carojeandat
Copy link
Contributor Author

There is a scroll bar now, can we avoid that? Only in english actually because it's on two lines.

this was already the case before (in the tab security analysis)

antoinebhs
antoinebhs approved these changes Sep 12, 2025
View reviewed changes
Copy link
Contributor

@antoinebhs antoinebhs left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Uniformisation of parameters will be handled separately

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@carojeandat carojeandat merged commit 7f69f1f into main Sep 12, 2025
5 checks passed
@carojeandat carojeandat deleted the change-limit-reductions-labels branch September 12, 2025 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sBouzols sBouzols sBouzols approved these changes

@antoinebhs antoinebhs antoinebhs approved these changes

@souissimai souissimai Awaiting requested review from souissimai

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@carojeandat @sBouzols @antoinebhs @souissimai