scijava / scijava-ui-swing

SciJava UI components for Java Swing.
BSD 2-Clause "Simplified" License
7 stars 11 forks source link

Improve number format in SwingNumberWidget #54

Closed imagejan closed 3 years ago

imagejan commented 4 years ago

This pull request depends on https://github.com/scijava/scijava-common/pull/405 and supersedes #52.

It includes the changes by @BoudewijnvanLangerak (in #52) as a squashed commit.

Changes can be tested with SwingNumberWidgetDemo added here, or with the following script:

// Automatically generated format
#@ Double (value=0.0000123, persist=false) a
#@ Double (value=0.000123, persist=false) b
#@ Double (value=0.00123, persist=false) c
#@ Double (value=0.0123, persist=false) d
#@ Double (value=0.123, persist=false) e
#@ Double (value=1.23, persist=false) f
#@ Double (value=123, persist=false) g
#@ Double (value=1, min=0.0, max=10.0, stepSize=0.001, persist=false) h

// Specified format
#@ Double (value=0.0123, persist=false, style="format:#.##") i
#@ Double (value=0.0123, persist=false, style="format:#.00") j
#@ Double (value=123.45, persist=false, style="format:#####.#####") k
#@ Double (value=123.45, persist=false, style="format:00000.00000") l

// Sliders and scroll bars
#@ Double (value=1, min=0, max=10, stepSize=0.001, persist=false, style=slider) m
#@ Double (value=1, min=0, max=10, stepSize=0.001, persist=false, style="slider,format:0.0000") n
#@ Double (value=1, min=0, max=10, stepSize=0.001, persist=false, style="scroll bar") o
#@ Double (value=1, min=0, max=10, stepSize=0.001, persist=false, style="scroll bar,format:0.0000") p

Here's a visual comparison before and after the changes in this commit:

Screenshot 2020-11-01 at 15 42 28 Screenshot 2020-11-01 at 16 07 49

(Fixes https://github.com/scijava/scijava-ui-swing/issues/45.)

imagesc-bot commented 3 years ago

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/save-one-tiff-projection-from-a-current-view-in-bdv/33965/28

NicoKiaru commented 3 years ago

@imagejan Quick question now that I managed to build it: currently is there a way to force / support scientific notation like 1.23e6 - at least in the display side ? I need it because for the same field, sometimes we can work in mm, sometimes in nm.

NicoKiaru commented 3 years ago

Hmm, this does not seem to work:

    @Parameter(style = "format:%6.3e", persist = false)
    private Double q = 0.0123;

image

NicoKiaru commented 3 years ago

But this works!

    @Parameter(style = "format:0.#####E0", persist = false)
    private Double q = 0.0123;

Ok, great!

imagejan commented 3 years ago

Glad you found a working option!

See also: https://stackoverflow.com/a/2944856/1919049 and https://javadoc.scijava.org/Java8/index.html?java/text/DecimalFormat.html

As well as this related issue: https://github.com/scijava/scijava-ui-swing/issues/2

NicoKiaru commented 3 years ago

Ok, just one thing, I tried a few things which can probably be improved. For a Double field:

Do you think we can at least get the display updated to the previous value when we type non valid characters ? Maybe it's a different issue than this PR ?

imagejan commented 3 years ago

I updated the pull request to depend on scijava-common-2.87.0-SNAPSHOT instead of a local 999 version, so that the CI build passes. We still need to remove the temporary version pin after a scijava-common-2.87.0 release gets cut.

There's one minor issue with the tests: on non-US locales, the test fails with:

[ERROR] Failures: 
[ERROR]   SwingNumberWidgetTest.test:97 Format (index 0) expected:<0[.]0000123> but was:<0[,]0000123>

@hinerm, @ctrueden: what's your opinion, should we always force a US locale in the Swing dialogs? That's how all the ImageJ 1.x dialogs do it, if I'm not mistaken. So it would help streamline the user experience...

NicoKiaru commented 3 years ago

@hinerm, @ctrueden: what's your opinion, should we always force a US locale in the Swing dialogs? That's how all the ImageJ 1.x dialogs do it, if I'm not mistaken. So it would help streamline the user experience...

Please do it !! Me and @tinevez (and many users) had terrible experiences with locale ... USA, USA! ;-)

imagejan commented 3 years ago

@ctrueden, @hinerm This pull request is now ready for review. I removed the SNAPSHOT coupling and pinned to the newly released scijava-common-2.87.0.

@NicoKiaru I tried forcing US locale in JSpinner.NumberEditor in https://github.com/scijava/scijava-ui-swing/commit/e044d64138549d03d826c2f71732c303994de641, but the test still fails on a system with non-US default locale, maybe I misunderstood how it's supposed to work?

NicoKiaru commented 3 years ago

@NicoKiaru I tried forcing US locale in JSpinner.NumberEditor in e044d64, but the test still fails on a system with non-US default locale, maybe I misunderstood how it's supposed to work?

Ok, let's try to live with that, I couldn't find a way either. And anyway maybe it's better to set the "Locale" more globally than in some widgets here and there. This PR is already a great improvement!