Skip to content

Conversation

@abigailalexander
Copy link
Collaborator

I have updated the GroupBox widget appearance, and pointed the widget mapping to correctly use GroupBox instead of GroupingContainer for bob files. None of the opi behaviour should be affected by these changes.

Changes:

  • Phoebus group boxes come in 4 styles - Group Box, Title Bar, Line and None. These are now all included. The style property is parsed as styleOpt, in order to prevent confusion when passing the prop to a component (not doing so triggers an eslint warning Style prop value must be an object)
  • Removed the compat property and set the style for GroupBox EmbeddedDisplays to the default groupbox style. This should give the same appearance as before
  • Props were not being passed to the widget correctly due to missing GroupBoxProps in GroupBoxWidgetProps. This has now been fixed
  • Updated tests

Copy link
Collaborator

@rjwills28 rjwills28 left a comment

Choose a reason for hiding this comment

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

Thanks for this PR - I didn't realise that the Group didn't work at all for BOB file. I've tested this and just found a few issues when experimenting with different Group properties in Phoebus, which I should now have fixed with my latest commits.

  1. If the name of the Group is missing (i.e. maybe you don't want a name at the top of the group) then the parser fails as the 'name' is a non-optional property. I've made this property optional and set the name as '' if it is missing.
    nameMissing

  2. The title bar color is incorrect and the border line color doesn't change (see below Phoebus cf cs-web).
    wrongColors
    Here the foreground color (i.e. text color) should be red and the line color should be yellow as shown on the left. On the right you can see the background of the title bar has been changed to the foreground color and the border has remained black. I fixed this by adding the 'line_color' property to the parser and using this to set the border color and the title bar color, while using the foreground color for the title bar title.

  3. The title in the title bar appears in the centre whereas in Phoebus it is on the left. I've just set the textAlign property in the css to fix that.

  4. If the foreground 'name' color is changed from black (e.g. in the case above it is red) then this propagates through to the other widgets contained in the group, i.e. in this case the label widgets turn red. I've fixed this by specifying the color black for the inner CSS. This will be overwritten if another color is specified by that widget (see below for example where the color of the label is purple) but if not specified will default to black.
    fixed

@abigailalexander
Copy link
Collaborator Author

Having updated Phoebus from 4.7.2 to 4.7.3 I am able to see the line_color property and can confirm these changes work and fix the issues in appearance highlighted

@rjwills28 rjwills28 self-requested a review February 18, 2025 13:44
Copy link
Collaborator

@rjwills28 rjwills28 left a comment

Choose a reason for hiding this comment

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

Approving if we're all happy with the changes.

@abigailalexander abigailalexander merged commit 91f0f8d into master Feb 18, 2025
2 checks passed
@abigailalexander abigailalexander deleted the update-groupbox-widget branch March 18, 2025 13:34
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