-
Notifications
You must be signed in to change notification settings - Fork 74
[web] [Storage] Allow setting/editing volume sizes #590
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
Merged
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
94cc4e1
[web] Enhance and extract the VolumeForm
dgdavid 9cb50eb
[web] Render sizes without trailing zeroes
dgdavid b4a7514
[web] Improve volume size column
dgdavid 4cb533c
[web] Add missing tests
dgdavid 2b330fb
[web] Use const for default size unit
dgdavid b183407
[web] Remove not needed key props
dgdavid e189a65
[web] Move mount point selector to internal widget
dgdavid a4b8595
[web] Small changes related to size unit selector
dgdavid 4bf7d6e
[web] Remove leftover
dgdavid 417f8bf
[web] Add a NumericTextInput helper component
dgdavid d822a80
[web] Remove empty line
dgdavid 58ca603
[web] Update changes file
dgdavid 8d19eec
[web] Improve how VolumesForm manage size units
dgdavid 47f6af6
Fix changelog from code review
dgdavid 8ef2d99
[web] Changes based on code review
dgdavid eb2d91d
[web] Fix filesystem type input
dgdavid File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| /* | ||
| * Copyright (c) [2023] SUSE LLC | ||
| * | ||
| * All Rights Reserved. | ||
| * | ||
| * This program is free software; you can redistribute it and/or modify it | ||
| * under the terms of version 2 of the GNU General Public License as published | ||
| * by the Free Software Foundation. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, but WITHOUT | ||
| * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or | ||
| * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for | ||
| * more details. | ||
| * | ||
| * You should have received a copy of the GNU General Public License along | ||
| * with this program; if not, contact SUSE LLC. | ||
| * | ||
| * To contact SUSE LLC about this file by physical or electronic mail, you may | ||
| * find current contact information at www.suse.com. | ||
| */ | ||
|
|
||
| import React from "react"; | ||
| import { TextInput } from "@patternfly/react-core"; | ||
| import { noop } from "~/utils"; | ||
|
|
||
| /** | ||
| * Callback function for notifying a valid input change | ||
| * | ||
| * @callback onChangeFn | ||
| * @param {string|number} the input value | ||
| * @return {void} | ||
| */ | ||
|
|
||
| /** | ||
| * Helper component for having an input text limited to not signed numbers | ||
| * @component | ||
| * | ||
| * Based on {@link PF/TextInput https://www.patternfly.org/v4/components/text-input} | ||
| * | ||
| * @note It allows empty value too. | ||
| * | ||
| * @param {object} props | ||
| * @param {string|number} props.value - the input value | ||
| * @param {onChangeFn} props.onChange - the callback to be called when the entered value match the input pattern | ||
| * @param {object} props.textInputProps - @see {@link https://www.patternfly.org/v4/components/text-input/#textinput} | ||
| * | ||
| * @returns {ReactComponent} | ||
| */ | ||
| export default function NumericTextInput({ value = "", onChange = noop, ...textInputProps }) { | ||
| // NOTE: Using \d* instead of \d+ at the beginning to allow empty | ||
| const pattern = /^\d*\.?\d*$/; | ||
|
|
||
| const handleOnChange = (value) => { | ||
| if (pattern.test(value)) { | ||
| onChange(value); | ||
| } | ||
| }; | ||
|
|
||
| return ( | ||
| <TextInput { ...textInputProps } value={value} onChange={handleOnChange} /> | ||
| ); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| /* | ||
| * Copyright (c) [2022-2023] SUSE LLC | ||
| * | ||
| * All Rights Reserved. | ||
| * | ||
| * This program is free software; you can redistribute it and/or modify it | ||
| * under the terms of version 2 of the GNU General Public License as published | ||
| * by the Free Software Foundation. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, but WITHOUT | ||
| * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or | ||
| * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for | ||
| * more details. | ||
| * | ||
| * You should have received a copy of the GNU General Public License along | ||
| * with this program; if not, contact SUSE LLC. | ||
| * | ||
| * To contact SUSE LLC about this file by physical or electronic mail, you may | ||
| * find current contact information at www.suse.com. | ||
| */ | ||
|
|
||
| import React, { useState } from "react"; | ||
| import { screen } from "@testing-library/react"; | ||
| import { plainRender } from "~/test-utils"; | ||
| import { NumericTextInput } from "~/components/core"; | ||
|
|
||
| // Using a controlled component for testing the rendered result instead of testing if | ||
| // the given onChange callback is called. The former is more aligned with the | ||
| // React Testing Library principles, https://testing-library.com/docs/guiding-principles | ||
| const Input = ({ value: initialValue = "" }) => { | ||
| const [value, setValue] = useState(initialValue); | ||
| return <NumericTextInput aria-label="Test input" value={value} onChange={setValue} />; | ||
| }; | ||
|
|
||
| it("renders an input text control", () => { | ||
| plainRender(<Input />); | ||
|
|
||
| const input = screen.getByRole("textbox", { name: "Test input" }); | ||
| expect(input).toHaveAttribute("type", "text"); | ||
| }); | ||
|
|
||
| it("allows only digits and dot", async () => { | ||
| const { user } = plainRender(<Input />); | ||
|
|
||
| const input = screen.getByRole("textbox", { name: "Test input" }); | ||
| expect(input).toHaveValue(""); | ||
|
|
||
| await user.type(input, "-"); | ||
| expect(input).toHaveValue(""); | ||
|
|
||
| await user.type(input, "+"); | ||
| expect(input).toHaveValue(""); | ||
|
|
||
| await user.type(input, "1"); | ||
| expect(input).toHaveValue("1"); | ||
|
|
||
| await user.type(input, ".5"); | ||
| expect(input).toHaveValue("1.5"); | ||
|
|
||
| await user.type(input, " GiB"); | ||
| expect(input).toHaveValue("1.5"); | ||
| }); | ||
|
|
||
| it("allows clearing the input (empty values)", async () => { | ||
| const { user } = plainRender(<Input value="120" />); | ||
|
|
||
| const input = screen.getByRole("textbox", { name: "Test input" }); | ||
| expect(input).toHaveValue("120"); | ||
| await user.clear(input); | ||
| expect(input).toHaveValue(""); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
We could go further and do this helper more generic by receiving the pattern via prop. But let's wait until we really need it. Do you agree?
BTW, we are not using directly the HTML pattern attribute because it's not intended for limiting the input from the user but for performing validations before send the form. But since we're preventing the form submission, it is not as direct as the proposed solution. To learn more, read https://developer.mozilla.org/en-US/docs/Web/HTML/Constraint_validation#constraint_validation_process
Of course, we can evaluate the use of
patternattribute in the future.