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.

Looks good! Most of the issues I'm seeing seem to be existing issues not ones related to your changes, such as:

  • Multi-line and text wrapping
  • In Phoebus you can set a border, it would be good to include this

It appears we were doing things quite weirdly in this widget, so your changes are much appreciated. Also don't forget the CSS file ;)

@NatLeung96
Copy link
Collaborator Author

NatLeung96 commented Apr 11, 2025

This has been a wild revision.

PV props
I'm hoping that I got the props type declarations properly set up like the other widgets. This did highlight that there may have been a bug with the definition of PVInputComponent type as pvName was declared twice with different props (one with PvPropOpt - i.e. PV - and the other with string). I took the string definition to be incorrect and changed that to PV. As a result, I had to change some variables and edit some tests. Hopefully that was the correct thing to do.

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.

The appearance looks really good now! Just the two comments below

Comment on lines 103 to 105
const [inputValue, setInputValue] = useState(
props.value?.getStringValue() ?? ""
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems like the value from a PV is always set to undefined (since that is what it first loads as). Adding a useEffect hook here which has value.getStringValue() passed in as the dependency value should fix this, as every time the string value is updated it will update inputValue (rather than leaving it stuck at the first value received from a PV, which is always undefined while the connection establishes itself).

Either that or recreate something similar to the old component in components/input/input.tsx with the editing state:

  if (!editing && inputValue !== props.value) {
    setInputValue(props.value);
  }

enabled: BoolPropOpt,
textAlign: ChoicePropOpt(["left", "center", "right"]),
textAlignV: ChoicePropOpt(["top", "center", "bottom"]),
border: BorderPropOpt
Copy link
Collaborator

Choose a reason for hiding this comment

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

You added multiLine into the bob parser, but it hasn't been included in this list as a prop or in the component itself.

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.

Value from PV shows as expected now, LGTM

@NatLeung96 NatLeung96 merged commit 71531e5 into 80-update-widgets-to-use-mui-base Apr 16, 2025
2 checks passed
@NatLeung96 NatLeung96 deleted the 91-replace-input-with-mui-textfield branch April 16, 2025 14:27
NatLeung96 added a commit that referenced this pull request Apr 22, 2025
* Replaced base input with MUI TextField

* Updated theme tokens and changed border colours

* Move static styling to styled API

* Added alignment props to parser

* Handle alignment props

* Updated test and created snapshot

* Removed redundant input component files

* Removed redundant reference to component file

* Removed redundant horizontalAlignment and verticalAlignment props from parser

* Added textAlignV as ChoicePropOpt and removed unnecessary switch statements

* Added if-else for vertical alignment

* Changed default background to teal (#80FFFF)

* Removed redundant border parsing

* Changed color of hover and focussed borders to blue

* Added custom border

* Added multiLine to parser

* Changed default blue border colour

* TextField displays existing PV value if available

* Reformatted prop type declaration and disabled cursor when disabled

* Updated tests, snapshots, and type definitions

* Updated slidecontrol test

* Updated input snapshot

* Corrected writePV function call

* Removed redundant CSS file

* Added blur on Enter

* Removed comment

* Got border thicknesses to change on hover and focus

* Changed border width values for hover and focussed

* Updated snapshot

* Reorganised widgetprop definitions and updated test

* Added useEffect to set initial text value when PV connection is established

* Updated pvName variable in slideControl

* Updated slideControl test

* Added multiline functionality
NatLeung96 added a commit that referenced this pull request Apr 23, 2025
* Added styling

* Setup placeholder until option is clicked

* 86 replace boolean button with mui button (#92)

* Replaced base html button with MUI Button

* Implemented disabled features and tidied up style parameters

* Moved static styling to styled API

* Commented out Button styling from tests

* Update tests and snapshots

* 90 replace label with mui typography (#93)

* Replaced div with MUI Typography

* Added styling props to Typography

* Updated the tests for label

* Updated symbols test

* Removed ThemeProvider and tidied styling parameters

* Updated tokens and fixed rotation

* Updated snapshots

* Removed redundant css file

* Moved static styling to styled API

* Updated tests and snapshots

* Set display to flex

* Updated snapshots

* Updated snapshot

* Replaced Checkbox with MUI Checkbox (#89)

* Replaced Checkbox with MUI Checkbox

* Removed ThemeProvider and added additional styling to FormControlLabel

* Moved static styling to styled API. Not-allowed cursor when widget is disabled. Added font styling.

* Removed padding

* Removed padding and fixed alignments

* Tidied nested for-loop

* Tidied up styling and prop definitions

* Added DType proptypes

* Updated snapshots and tests

* Changed menuButton logics and added items as potential list of options

* Added useEffect to prevent re-render errors

* Updated tests and snapshot

* 87 replace choice button with mui button group (#98)

* Replaced base react button group with MUI ToggleButtonGroup

* Commented out expect statements checking the styling

* Removed ThemeProvider

* Moved static styling to styled API

* Updated tests

* Removed redundant height and width for ButtonGroup

* Set opacity to 0.6 for hover and selected-hover

* Added borders to buttons and fixed text wrapping

* Updated snapshots

* Deleted redundant CSS file

* Removed CSS import

* Changed button height/width and hover opacity

* Updated snapshots

* 95 replace progress bar with mui progress bar (#100)

* Replaced divs with LinearProgress

* Added width and height to parser

* Implemented vertical orientation

* Implements default width and height if not provided

* Fixed width and height checking

* Fixed label and removed redundant css file

* Moved default width and height declarations to function head

* Removed redundant height and width from bob parser

* Corrected percentage calculation

* Added transparency option

* Added check for out of range values

* 91 replace input with mui textfield (#99)

* Replaced base input with MUI TextField

* Updated theme tokens and changed border colours

* Move static styling to styled API

* Added alignment props to parser

* Handle alignment props

* Updated test and created snapshot

* Removed redundant input component files

* Removed redundant reference to component file

* Removed redundant horizontalAlignment and verticalAlignment props from parser

* Added textAlignV as ChoicePropOpt and removed unnecessary switch statements

* Added if-else for vertical alignment

* Changed default background to teal (#80FFFF)

* Removed redundant border parsing

* Changed color of hover and focussed borders to blue

* Added custom border

* Added multiLine to parser

* Changed default blue border colour

* TextField displays existing PV value if available

* Reformatted prop type declaration and disabled cursor when disabled

* Updated tests, snapshots, and type definitions

* Updated slidecontrol test

* Updated input snapshot

* Corrected writePV function call

* Removed redundant CSS file

* Added blur on Enter

* Removed comment

* Got border thicknesses to change on hover and focus

* Changed border width values for hover and focussed

* Updated snapshot

* Reorganised widgetprop definitions and updated test

* Added useEffect to set initial text value when PV connection is established

* Updated pvName variable in slideControl

* Updated slideControl test

* Added multiline functionality

* Removed unnecessary DTypeProp types

* Removed unnecessary props from MenuButtonComponentProps

* Removed unnecessary import of DType
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