Skip to content

Conversation

@NatLeung96
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@abigailalexander abigailalexander left a comment

Choose a reason for hiding this comment

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

I can see the arrowheads working properly now for the rules, great! I had one or two minor CSS/Phoebus differences that would be good to be implemented.

fillArrow = true
} = props;

const color = transparent ? "transparent" : backgroundColor.toString();
Copy link
Collaborator

Choose a reason for hiding this comment

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

The use of background colour here is part of CSStudio. In Phoebus, we no longer have a background colour (or transparent) but use lineColour instead. I'd suggest updating this to check for lineColor first, then doing the transparent/backgroundColor set next so we can cover both the CSS and phoebus use cases.

height = WIDGET_DEFAULT_SIZES["polyline"][1],
lineWidth = 3,
points,
arrowLength = 2,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is another "still using the old CSS values" issue, but the default arrow length is now 20. If you load a file that uses default arrow size 20 you can't see the arrow in the web because it is too small.

? Color.TRANSPARENT.toString()
: backgroundColor?.toString()
}
fill={"none"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This fill="none" property should be kept, otherwise I see a black background on the widget.
Screenshot from 2025-05-06 09-04-54

@NatLeung96 NatLeung96 merged commit 84c94c8 into 80-update-widgets-to-use-mui-base May 6, 2025
2 checks passed
@NatLeung96 NatLeung96 deleted the 112-fix-arrow-markers-on-line-widget branch May 6, 2025 09:45
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