Skip to content

Conversation

@flomillot
Copy link
Contributor

@flomillot flomillot commented Dec 26, 2025

PR Summary

This pull request refactors the global filter to support voltage ranges. Also, the method getBaseVoltage is renamed to getBaseVoltageInterval.

- Rename `getBaseVoltage` to `getBaseVoltageInterval`
- Update all relevant components and hooks to use voltage ranges and new method naming

Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
…ent and key cleanup

Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
@sonarqubecloud
Copy link

Copy link
Contributor

@Mathieu-Deharbe Mathieu-Deharbe left a comment

Choose a reason for hiding this comment

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

OK

<Checkbox size="small" checked={state.selected} />
{option.filterType === FilterType.VOLTAGE_LEVEL ? (
<Tooltip title={formatVoltageRange(option)} placement="right">
<Typography>{label}</Typography>
Copy link
Contributor

@Mathieu-Deharbe Mathieu-Deharbe Jan 7, 2026

Choose a reason for hiding this comment

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

For now the titles are small but wouldn't it be more careful to use a OverflowableText just in case ? Just like the others. After all this part is already partly customizable by the user.

<ListItemButton selected={state.selected} component="li" {...otherProps}>
<Checkbox size="small" checked={state.selected} />
{option.filterType === FilterType.VOLTAGE_LEVEL ? (
<Tooltip title={formatVoltageRange(option)} placement="right">
Copy link
Contributor

Choose a reason for hiding this comment

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

The last time I added a tooltip Stéphane asked for an arrow on it. I like it :

Suggested change
<Tooltip title={formatVoltageRange(option)} placement="right">
<Tooltip title={formatVoltageRange(option)} placement="right" arrow>

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.

3 participants