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

NCD-776: Limit voltage presets to fit the boards range #73

Merged
merged 4 commits into from
Mar 7, 2024

Conversation

cybic
Copy link
Contributor

@cybic cybic commented Mar 7, 2024

  • Filtering out inapplicable voltage presets
  • Refactor voltage preset buttons to functional components
  • Make ready for preset values from board definition file

@cybic cybic added doc not required All PRs either need "doc required" or "doc not required". ui not required All PRs either need "ui required" or "ui not required". labels Mar 7, 2024
@cybic cybic requested a review from boundlesscalm March 7, 2024 09:31
Comment on lines 44 to 50
const voltagePresetValues = [1200, 1800, 2000, 3300, 1600, 2400]
.filter(
filterVoltage =>
filterVoltage >= voltageMin && filterVoltage <= voltageMax
)
.slice(0, 3)
.sort((a, b) => a - b);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not that it really matters, but why have unsorted presets only to explicitly sort them after?

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 does, in fact, matter :) The reason is to pick relevant voltage levels fro the different voltage ranges. I slice of the tree first elements to pick the most relevant fitting the range, and finally sort them to make the UI nice.

Good catch, though! 👍

@cybic cybic merged commit 829309e into main Mar 7, 2024
3 checks passed
@cybic cybic deleted the NCD-776_limit_voltage_presets_to_boards_range branch March 7, 2024 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc not required All PRs either need "doc required" or "doc not required". ui not required All PRs either need "ui required" or "ui not required".
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants