Refined data plane view w/ dialog #1842
Conversation
| <Stack direction="row" spacing={1}> | ||
| <TextField | ||
| value={value || ''} | ||
| disabled | ||
| size="small" | ||
| fullWidth | ||
| sx={{ | ||
| 'flex': 1, | ||
| '& .MuiInputBase-input': { | ||
| fontWeight: 500, | ||
| fontFamily: 'Monospace', | ||
| fontSize: 12, | ||
| }, | ||
| }} | ||
| /> | ||
| <CopyToClipboardButton writeValue={value} /> | ||
| </Stack> |
There was a problem hiding this comment.
I think we could just use SingleLineCode here and get generally the same experience. It looks much more like a box and does not follow the design principles of our forms but a somewhat common pattern for values users might need to copy.
There was a problem hiding this comment.
My thinking is to differentiate from form fields that the user has to edit vs uneditable values to copy
| value={row.gcp_service_account_email} | ||
| /> | ||
| ) : null} | ||
| <TechnicalEmphasis>{splitCidrBlocks.ipv4}</TechnicalEmphasis> |
There was a problem hiding this comment.
What is the UX through behind making these no longer easy to copy? So we can easily tell the user to always open the dialog and everything they could need to copy is there?
There was a problem hiding this comment.
The idea is to simplify the table UX - easier to parse, and clicking anywhere on a row does a single thing: opens the dialog box
travjenkins
left a comment
travjenkins
left a comment
There was a problem hiding this comment.
things need broken apart and some areas we can reuse already existing components.
Remove hideOnHover prop from CopyIconIndicator and move the conditional logic to the parent component.
|
Added the "private" badge 
|
There was a problem hiding this comment.
We don't really use barrel files. You can just import directly.
There was a problem hiding this comment.
I'll remove it, but I like that it communicates that some of the internal exported components aren't necessarily designed for use outside of this directory (like the CopyIconIndicator - maybe it's useful elsewhere but I haven't put that thought into it yet)
I also appreciate this grouping
import {
DataPlaneDialogField,
ToggleField,
} from 'src/components/tables/DataPlanes/DialogFields';There was a problem hiding this comment.
(i get this is basically philosophical, just curious if there's reasoning behind the convention)
| case 'error': | ||
| return ( | ||
| <WarningCircle style={{ color: theme.palette.error.main }} /> | ||
| ); |
There was a problem hiding this comment.
What is the thought behind removing the ability to visually show the use an error? Just that we're not really expecting it to ever happen?
There was a problem hiding this comment.
basically yes - happy to add this back in if we start seeing failures in the logs, but YAGNI i think?
There was a problem hiding this comment.
I would leave it in there while we have the event and then if we don't see it for a month we can remove this support.
There was a problem hiding this comment.
okay - i put it back in there
travjenkins
left a comment
travjenkins
left a comment
There was a problem hiding this comment.
I think this should be the last round of changes
| export const logRocketEvent = ( | ||
| event: CustomEvents | KnownEvents | string, | ||
| // (string & {}) preserves autocomplete for CustomEvents and KnownEvents while allowing arbitrary strings | ||
| event: CustomEvents | KnownEvents | (string & {}), |
There was a problem hiding this comment.
(string & {}) gives us back this autocomplete
Issues
#1839
Changes
Tests
Manually tested
Automated tests
Playwright tests ran locally
Screenshots
Screen.Recording.2025-12-11.at.4.52.33.PM.mov