mgr/dashboard: Print a blank space between value and unit #22387
Conversation
votdev
requested review from
LenzGr,
tspmelo,
Exotelis,
ricardoasmarques and
Devp00l
votdev
changed the title
|
retest this please |
votdev
force-pushed
the
missing_space_value_unit
branch
2 times, most recently
from
e0a0f27
to
3b64a3d
Compare
| return '-'; | ||
| } | ||
| let result = truncatedFloat; | ||
| if (units[unit] !== '') { |
There was a problem hiding this comment.
This can only happen if the units array has an empty string in it.
It would be better to check the following:
units[units.length <= unit ? units.length -1 : unit]
| if (truncatedFloat === '') { | ||
| return '-'; | ||
| } | ||
| let result = truncatedFloat; |
There was a problem hiding this comment.
Maybe you should rename truncatedFloat to result in line 31 ;)
There was a problem hiding this comment.
I was thinking about this during the implementation phase, but decided not to
name it as you suggested because it is not the result at this stage.
votdev
force-pushed
the
missing_space_value_unit
branch
from
3b64a3d
to
f1b3b90
Compare
|
@Devp00l All comments have beem addressed. |
votdev
force-pushed
the
missing_space_value_unit
branch
2 times, most recently
from
56f7519
to
9eb0b34
Compare
| if (result === '') { | ||
| return '-'; | ||
| } | ||
| if (units[unit] !== '') { |
There was a problem hiding this comment.
I think you can now remove this line :)
This said how about converting it to a one liner :D
return result === '' ? '-' : `${result} ${units[unit]}`
There was a problem hiding this comment.
We still need that line because the list of units can contain an empty unit string, see https://github.com/ceph/ceph/blob/master/src/pybind/mgr/dashboard/frontend/src/app/shared/pipes/dimless.pipe.ts#L12.
votdev
force-pushed
the
missing_space_value_unit
branch
from
9eb0b34
to
ba55d24
Compare
When displaying values with their unit a blank space must be shown between them, see #15 at https://physics.nist.gov/cuu/Units/checklist.html. Signed-off-by: Volker Theile <vtheile@suse.com>
votdev
force-pushed
the
missing_space_value_unit
branch
from
ba55d24
to
bc44383
Compare
When displaying values with their unit a blank space must be shown between them, see https://en.wikipedia.org/wiki/Space_(punctuation)#Unit_symbols and point 15 at https://physics.nist.gov/cuu/Units/checklist.html.
Signed-off-by: Volker Theile vtheile@suse.com