-
Notifications
You must be signed in to change notification settings - Fork 16
Show image size on image picker; clean up empty metadata; small color fix #1836
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ import { useController, type Control } from 'react-hook-form' | |
|
|
||
| import type { Image } from '@oxide/api' | ||
| import type { ListboxItem } from '@oxide/ui' | ||
| import { GiB } from '@oxide/util' | ||
| import { bytesToGiB, GiB } from '@oxide/util' | ||
|
|
||
| import type { InstanceCreateInput } from 'app/forms/instance-create' | ||
|
|
||
|
|
@@ -41,22 +41,43 @@ export function ImageSelectField({ images, control }: ImageSelectFieldProps) { | |
| ) | ||
| } | ||
|
|
||
| const Slash = () => <span className="mx-0.5 text-quinary">/</span> | ||
| const Slash = () => ( | ||
| <span className="mx-1 text-quinary selected:text-accent-disabled">/</span> | ||
| ) | ||
|
|
||
| export function toListboxItem(i: Image, includeProjectSiloIndicator = false): ListboxItem { | ||
| const projectSiloIndicator = includeProjectSiloIndicator ? ( | ||
| <> | ||
| <Slash /> {i.projectId ? 'Project image' : 'Silo image'} | ||
| </> | ||
| ) : null | ||
| const { name, os, projectId, size, version } = i | ||
| const formattedSize = `${bytesToGiB(size, 1)} GiB` | ||
|
|
||
| // filter out any undefined metadata and create a comma-separated list | ||
| // for the selected listbox item (shown in labelString) | ||
| const condensedImageMetadata = [os, version, formattedSize].filter((i) => !!i).join(', ') | ||
| const metadataForLabelString = condensedImageMetadata.length | ||
| ? ` (${condensedImageMetadata})` | ||
| : '' | ||
|
|
||
| // for metadata showing in the dropdown's options, include the project / silo indicator if requested | ||
| const projectSiloIndicator = includeProjectSiloIndicator | ||
| ? `${projectId ? 'Project' : 'Silo'} image` | ||
| : null | ||
| // filter out undefined metadata here, as well, and create a `<Slash />`-separated list | ||
| // for the listbox item (shown for each item in the dropdown) | ||
| const metadataForLabel = [os, version, formattedSize, projectSiloIndicator] | ||
| .filter((i) => !!i) | ||
| .map((i, index) => ( | ||
| <span key={`${i}`}> | ||
| {index > 0 ? <Slash /> : ''} | ||
| {i} | ||
| </span> | ||
| )) | ||
| return { | ||
| value: i.id, | ||
| labelString: `${i.name} (${i.os}, ${i.version})`, | ||
| labelString: `${name}${metadataForLabelString}`, | ||
| label: ( | ||
| <> | ||
| <div>{i.name}</div> | ||
| <div className="text-secondary"> | ||
| {i.os} <Slash /> {i.version} {projectSiloIndicator} | ||
| <div>{name}</div> | ||
| <div className="text-tertiary selected:text-accent-secondary"> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small tweak to match component in Figma library |
||
| {metadataForLabel} | ||
| </div> | ||
| </> | ||
| ), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,20 @@ | |
| */ | ||
| import { expect, it } from 'vitest' | ||
|
|
||
| import { splitDecimal } from './math' | ||
| import { GiB } from '.' | ||
| import { round, splitDecimal } from './math' | ||
|
|
||
| it('rounds properly', () => { | ||
| expect(round(123.456, 0)).toEqual(123) | ||
| expect(round(123.456, 1)).toEqual(123.5) | ||
| expect(round(123.456, 2)).toEqual(123.46) | ||
| expect(round(123.456, 3)).toEqual(123.456) | ||
| expect(round(123.456, 4)).toEqual(123.456) // trailing zeros are culled | ||
| expect(round(1.9, 0)).toEqual(2) | ||
| expect(round(1.9, 1)).toEqual(1.9) | ||
| expect(round(5 / 2, 2)).toEqual(2.5) // math expressions are resolved | ||
| expect(round(1879048192 / GiB, 2)).toEqual(1.75) // constants can be evaluated | ||
| }) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we stan a testing king
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Haha, I was mostly trying to figure out why my 1.2GiB image kept getting represented as 2GiB, and hadn't yet looked at the API response. This is probably ultimately just testing Math.round, but I figured we could keep it around. |
||
|
|
||
| it.each([ | ||
| [1.23, ['1', '.23']], | ||
|
|
||
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.