chore: move ui kit to separate repo #8104
Conversation
✅ Deploy Preview for determined-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -165,6 +165,7 @@ const ExperimentMultiTrialTabs: React.FC<Props> = ({ | |||
| disableTitle | |||
| notes={{ contents: experiment.notes ?? '', name: 'Notes' }} | |||
| onError={handleError} | |||
| onPageUnloadHook={unstable_useBlocker} | |||
There was a problem hiding this comment.
the Note component that NoteCards renders should prompt the user when leaving the page if they're sure they want to leave if the note in question has dirty state. Before the split, Note imported unstable_useBlocker directly from react-router-dom to do the prompt, but that requires a data router in the component context, so now we pass the hook in from the caller via onPageUnloadHook.
| import { XOR } from 'determined-ui/internal/types'; | ||
| import usePrevious from 'determined-ui/internal/usePrevious'; |
There was a problem hiding this comment.
nit: we shouldn't be importing from internal, so these should either be duplicated in this repo or exposed more officially. in my Icon pr I chose to duplicate XOR, but exposing some utility types/hooks in determined-ui sounds like a good plan
| @@ -1,8 +1,8 @@ | |||
| import { Alert } from 'antd'; | |||
| import Message from 'determined-ui/internal/Message'; | |||
There was a problem hiding this comment.
nit: again about internal, though John's Message PR should fix this one
| @@ -1,10 +1,10 @@ | |||
| import Icon from 'determined-ui/Icon'; | |||
| import { DetError } from 'determined-ui/internal/types'; | |||
| @@ -1,7 +1,7 @@ | |||
| import { LineChart } from 'determined-ui/LineChart'; | |||
There was a problem hiding this comment.
super nit: nothing to do with this page specifically, but it feels like LineChart should be the default export from determined-ui/LineChart
| import LogViewerSelect, { Filters } from 'determined-ui/LogViewer/LogViewerSelect'; | ||
| import { Settings, settingsConfigForTrial } from 'determined-ui/LogViewer/LogViewerSelect.settings'; |
There was a problem hiding this comment.
nit: maybe re-export the settings from the main file?
ashtonG
force-pushed
the
chore/WEB-1687/ui-kit-module
branch
2 times, most recently
from
ab86cd3
to
7c5abff
Compare
This moves the source of the ui kit to a separate repo at determined-ai/determined-ui. the old UI kit is removed and all imports are updated. The new UI kit doesn't currently have CI or netlify set up (we need to set up another set of github credentials so the differ works), so the design kit and differ will continue to run here while that gets up and going.
ashtonG
force-pushed
the
chore/WEB-1687/ui-kit-module
branch
from
c22b331
to
c54daba
Compare
Description
This moves the source of the ui kit to a separate repo at determined-ai/determined-ui. the old UI kit is removed and all imports are updated. The new UI kit doesn't currently have CI or netlify set up (we need to set up another set of github credentials so the differ works), so the design kit and differ will continue to run here while that gets up and going.
Test Plan
Code behavior should be unchanged:
Commentary (optional)
because this is being installed from github, the package is built on install -- we should perhaps move to publishing this on the npm registry behind an organization (which will allow organizational control over who pushes new versions).
Checklist
docs/release-notes/.See Release Note for details.
Ticket
WEB-1687