-
Notifications
You must be signed in to change notification settings - Fork 45
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
UI: Add the ability to create LVM Logical Volume from Metalk8s UI #3410
UI: Add the ability to create LVM Logical Volume from Metalk8s UI #3410
Conversation
7fffb91
to
7d77a8a
Compare
341a2f7
to
37c898b
Compare
3711237
to
95a7986
Compare
ui/src/containers/CreateVolume.js
Outdated
@@ -711,7 +732,7 @@ const CreateVolume = (props) => { | |||
/> | |||
</SizeUnitFieldSelectContainer> | |||
</SizeFieldContainer> | |||
) : ( | |||
) : values.type === RAW_BLOCK_DEVICE ? ( |
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.
Now that there is more than 2 options I suggest to refactor this to a switch case
ui/src/containers/CreateVolume.js
Outdated
<> | ||
<SizeFieldContainer> | ||
<Input | ||
name="sizeInput" | ||
type="number" | ||
min="1" | ||
value={values.sizeInput} | ||
onChange={handleChange('sizeInput')} | ||
label={`${intl.translate('volume_size')}*`} | ||
error={touched.sizeInput && errors.sizeInput} | ||
onBlur={handleOnBlur} | ||
/> | ||
<SizeUnitFieldSelectContainer> | ||
<Input | ||
clearable={false} | ||
type="select" | ||
options={optionsSizeUnits} | ||
noOptionsMessage={() => | ||
intl.translate('no_results') | ||
} | ||
name="selectedUnit" | ||
onChange={handleSelectChange('selectedUnit')} | ||
value={getSelectedObjectItem( | ||
optionsSizeUnits, | ||
values?.selectedUnit, | ||
)} | ||
error={ | ||
touched.selectedUnit && errors.selectedUnit | ||
} | ||
onBlur={handleOnBlur} | ||
/> | ||
</SizeUnitFieldSelectContainer> | ||
</SizeFieldContainer> | ||
<Input | ||
name="vgName" | ||
value={values.vgName} | ||
onChange={handleChange('vgName')} | ||
label={`Volume Group Name*`} | ||
error={touched.vgName && errors.vgName} | ||
onBlur={handleOnBlur} | ||
/> | ||
</> |
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.
Maybe we can extract the fields to a component ? It would then make sense to validate those additional fields in that component and just add an onChange handler prop to that component that just addup the fields value to the submitted object.
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.
Hi @JBWatenbergScality
I am not sure of the benefit to extract these fields. For me, it would add some complexity than keep everything centralized. ( but I am open to discuss).
However, I see a reusable combined field sizeInputCombinedField
which is used in both SPARSE_LOOP_DEVICE
and LVM_LOGICAL_VOLUME
8cd3fda
Hello chengyanjin,My role is to assist you with the merge of this Status report is not available. |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list:
|
ConflictThere is a conflict between your branch Please resolve the conflict on the feature branch ( $ git fetch
$ git checkout origin/improvement/add-lvmvolume-creation-support
$ git merge origin/development/2.10
$ # <intense conflict resolution>
$ git commit
$ git push origin HEAD:improvement/add-lvmvolume-creation-support |
04adea5
to
9e0ba21
Compare
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list:
|
/approve |
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve |
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue None. Goodbye chengyanjin. |
Component: UI
Context: Add the ability to create LVM Logical Volume from the UI
Summary:
For the batch creation, we only implement the most simple case, in which we can only specify the Name between the volumes.
Add interaction test for this kind of volume.
Next step:
We should display a message/notification when the volume is failed to create, for example, the Volume Group doesn't exist.
More generally speaking, we will organise a design workshop with @Cuervino and maybe @JBWatenbergScality @MonPote to decide how to display the message when a failure popup.
Acceptance criteria:
Closes: #3244