-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move storage pool creation form from side panel to dedicated page [WD-6352] #493
Conversation
Demo starting at https://lxd-ui-493.demos.haus |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
a169d73
to
403d73c
Compare
@piperdeck Could you please do a design review when you have time? Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The demo build fails:
#14 8.763 [vite]: Rollup failed to resolve import "pages/instances/forms/FormMenuItem" from "/srv/src/pages/storage/forms/StoragePoolFormMenu.tsx".
#14 8.763 This is most likely unintended because it can break your application at runtime.
#14 8.763 If you do want to externalize this module explicitly add it to
#14 8.763 `build.rollupOptions.external`
#14 8.770 error during build:
#14 8.770 Error: [vite]: Rollup failed to resolve import "pages/instances/forms/FormMenuItem" from "/srv/src/pages/storage/forms/StoragePoolFormMenu.tsx".
#14 8.770 This is most likely unintended because it can break your application at runtime.
#14 8.770 If you do want to externalize this module explicitly add it to
#14 8.770 `build.rollupOptions.external`
#14 8.770 at viteWarn (file:///srv/node_modules/vite/dist/node/chunks/dep-bb8a8339.js:48216:27)
#14 8.770 at onwarn (/srv/node_modules/@vitejs/plugin-react/dist/index.cjs:255:9)
#14 8.770 at onRollupWarning (file:///srv/node_modules/vite/dist/node/chunks/dep-bb8a8339.js:48245:9)
#14 8.770 at onwarn (file:///srv/node_modules/vite/dist/node/chunks/dep-bb8a8339.js:47976:13)
#14 8.770 at file:///srv/node_modules/vite/node_modules/rollup/dist/es/shared/node-entry.js:24276:13
#14 8.770 at Object.logger [as onLog] (file:///srv/node_modules/vite/node_modules/rollup/dist/es/shared/node-entry.js:25950:9)
#14 8.770 at ModuleLoader.handleInvalidResolvedId (file:///srv/node_modules/vite/node_modules/rollup/dist/es/shared/node-entry.js:24862:26)
#14 8.770 at file:///srv/node_modules/vite/node_modules/rollup/dist/es/shared/node-entry.js:24822:26
I guess something went bad on the merge resolve?
Edit: The import needs to pick from components/forms
.
Edit: Pushed a fix to your branch @lorumic
780a2d2
to
53cd0fb
Compare
Demo is up and running with all the updates. |
The only issue I can find is that the description field should be a textarea, rather than a single-line field |
Good catch! I fixed that, and I also aligned all the description fields in other pages, which were still using the text input, to use textarea instead. Then I extracted the function to calculate the textarea's |
ac9f861
to
bfef3eb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks :)
Done
Fixes WD-6352
QA