Skip to content

Conversation

@ayolab
Copy link
Contributor

@ayolab ayolab commented Sep 16, 2025

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)

What kind of change does this PR introduce?
A feature update adding customizable limit-violation styling logic

What is the current behavior?
Branch and edge styles rely solely on built-in overload detection without recognizing externally provided styles.

What is the new behavior (if this is a feature change)?
Both TopologicalStyleProvider and LimitHighlightStyleProvider now accept limitViolationStyles maps that supply explicit classes per equipment; when absent or blank they fall back to the existing overload-based styling

Does this PR introduce a breaking change or deprecate an API?

  • Yes
  • No

Signed-off-by: Ayoub LABIDI <[email protected]>
Signed-off-by: Ayoub LABIDI <[email protected]>
@ayolab ayolab force-pushed the ayolab/add-limit-violation-custom-highlighting-support branch from 9249e32 to 574e7f2 Compare September 26, 2025 14:57
@sonarqubecloud
Copy link

@ayolab ayolab requested a review from SlimaneAmar September 26, 2025 15:15
// Check custom violations first
if (!limitViolationStyles.isEmpty()) {
String customStyle = limitViolationStyles.get(branchEdge.getEquipmentId());
if (customStyle != null && !customStyle.isBlank()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

StringUtils.isBlank() ?

*/
public class TopologicalStyleProvider extends AbstractVoltageStyleProvider {

private Map<String, String> limitViolationStyles = Map.of();
Copy link
Contributor

Choose a reason for hiding this comment

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

limitViolationStylesByBranchId ?

for (Node node : edge.getNodes()) {
if (node instanceof FeederNode feederNode) {
String style = limitViolationStyles.get(feederNode.getEquipmentId());
if (style != null && !style.isBlank()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

StringUtils.isBlank() ?

*/
public class LimitHighlightStyleProvider extends EmptyStyleProvider {
private final Network network;
private Map<String, String> limitViolationStyles = Map.of();
Copy link
Contributor

Choose a reason for hiding this comment

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

limitViolationStylesByBranchId ?

@github-project-automation github-project-automation bot moved this from In Progress to Approved in Release 09/2025 Sep 29, 2025
*/
public class LimitHighlightStyleProvider extends EmptyStyleProvider {
private final Network network;
private Map<String, String> limitViolationStyles = Map.of();
Copy link
Collaborator

@flo-dup flo-dup Oct 21, 2025

Choose a reason for hiding this comment

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

there's a problem here, there's nothing which links these styles to limit violations here. Why didn't you include the logic which gives a style to a specific violation limit in this class? Similarly to isOverloaded method.
If you keep that map you should rather call it something like overrideStyles, but then why do include this in a LimitHighlight class? That could be in a separate one.

@github-project-automation github-project-automation bot moved this from Approved to Waiting for review in Release 09/2025 Oct 21, 2025
@flo-dup flo-dup moved this from Waiting for review to In Progress in Release 09/2025 Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants