Add new selection syntax to job op graphs #26836
Conversation
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit c1aaa62. |
salazarm
force-pushed
the
salazarm/add-feature-flags
branch
from
ec095cb to
c1aaa62
Compare
| const names = new Set<string>(); | ||
| items.forEach((item) => { | ||
| names.add(item.name); | ||
| }); |
There was a problem hiding this comment.
⛳
const names = new Set(items.map(({name}) => name));
There was a problem hiding this comment.
I like having the for loop for the future in case we add more attributes and to be consistent with all the other inputs.
|
|
||
| const Wrapper = styled.div` | ||
| ${InputDiv} { | ||
| width: 24vw; |
There was a problem hiding this comment.
Does this need a min or max width to keep it looking sane in narrow or wide viewports?
There was a problem hiding this comment.
This is the existing width
which I just copied overThere was a problem hiding this comment.
Meant to link here
There was a problem hiding this comment.
Sounds reasonable, just figured I'd point it out in case it would look nicer with some upper/lower bounds.
Summary & Motivation
flagOpSelectionSyntaxintoflagSelectionSyntaxHow I Tested These Changes