Update field selection table row actions #1506
Conversation
There was a problem hiding this comment.
Not for this PR - but for the field selection overhaul in general.
I think we should switch all these functions over to objects we just store the settings in.
There was a problem hiding this comment.
This thought crossed my mind as well.
There was a problem hiding this comment.
I'd say the spirit of this is now generally done with the new alpha colors.
| requireOnly || | ||
| (excludeOnly && !recommendFields) |
There was a problem hiding this comment.
I think if a button is disabled here we should probably show a tooltip?
| [`&.${toggleButtonClasses.disabled}`]: selected | ||
| ? { | ||
| backgroundColor: theme.palette[colorKey].alpha_05, | ||
| border: `1px solid ${theme.palette[colorKey].alpha_12}`, | ||
| color: theme.palette[colorKey].alpha_26, | ||
| } | ||
| : { | ||
| backgroundColor: 'none', | ||
| border: defaultOutline[theme.palette.mode], | ||
| color: disabledButtonText[theme.palette.mode], | ||
| }, |
There was a problem hiding this comment.
What is the case where selected is true but the class for selected is not applied? Hoping we could get the same styling with more static styling.
There was a problem hiding this comment.
This should eventually live somewhere else. We probably want to put all the styled stuff in a single spot near the theme (and break that thing up). We should also probably just use our own ToggleButton that can be outlined or not and then stop using the MUI one all together.
| onClick={() => { | ||
| const singleValue = | ||
| selection?.mode !== 'default' ? 'default' : null; | ||
|
|
||
| const selectionType = evaluateSelectionType( | ||
| recommendFields[bindingUUID], | ||
| 'default', | ||
| selection?.mode ?? null, | ||
| singleValue | ||
| ); | ||
|
|
||
| setSingleSelection( | ||
| bindingUUID, | ||
| field, | ||
| selectionType, | ||
| selection?.meta | ||
| ); | ||
| }} |
There was a problem hiding this comment.
I think all these onClicks should be shared.
There was a problem hiding this comment.
They should also probably use the value from the function args.
|
|
||
| <OutlinedToggleButton | ||
| color="warning" | ||
| selected={selection?.mode === 'require'} |
There was a problem hiding this comment.
Can we use the `value' prop on the group so we don't have to manage this ourselves?
Issues
The issues directly below are completely resolved by this PR:
#1503
Changes
1503
The following features are included in this PR:
Replace Include row action with Require and Select row actions.
Order the row actions as follows starting from left to right: exclude, select, require.
Apply the styling of the former Include row action to the new Require and Select row actions.
Initialize a field according to the following logic:
A field is selected if the
recommendedflag is true and the field is not listed in theinclude/requireobject orexcludearray in the binding'sfieldsstanza.A field is required if is listed in the
include/requireobject in the binding'sfieldsstanza.A field is excluded if is listed in the
excludearray in the binding'sfieldsstanza.Tests
Manually tested
Approaches to testing are as follows:
Automated tests
Playwright tests ran locally
Screenshots
Field selection initialized |
recommendedtrue | No existing selectionsField selection initialized |
recommendedfalse | No existing selections