Skip to content

Improve number format in SwingNumberWidget #54

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 4 commits into from
Sep 13, 2021
Merged

Conversation

imagejan
Copy link
Member

@imagejan imagejan commented Nov 1, 2020

This pull request depends on scijava/scijava-common#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 #45.)

@imagesc-bot
Copy link

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
Copy link
Contributor

@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
Copy link
Contributor

Hmm, this does not seem to work:

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

image

@NicoKiaru
Copy link
Contributor

But this works!

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

Ok, great!

@imagejan
Copy link
Member Author

imagejan commented Sep 8, 2021

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: #2

@NicoKiaru
Copy link
Contributor

NicoKiaru commented Sep 8, 2021

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

  • You type 1E3 -> the display is modified into 1E3, the output value is 1000 // Ok!
  • You type 1e3 -> the display is modified into 1E0, the output value is 1 // sort of makes sense, the user realizes there's an issue with non capital E
  • You type GFP -> the display is modified into GFP, the output value is the previous value entered (default) // No error message. No modification in the display

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
Copy link
Member Author

imagejan commented Sep 8, 2021

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
Copy link
Contributor

@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 imagejan force-pushed the number-widget-format branch from ecf5184 to 95df845 Compare September 9, 2021 18:44
@imagejan imagejan marked this pull request as ready for review September 9, 2021 19:08
@imagejan
Copy link
Member Author

imagejan commented Sep 9, 2021

@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 e044d64, but the test still fails on a system with non-US default locale, maybe I misunderstood how it's supposed to work?

@NicoKiaru
Copy link
Contributor

@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!

@imagejan imagejan requested a review from hinerm September 13, 2021 13:22
@hinerm hinerm force-pushed the number-widget-format branch from 95df845 to d89b7c9 Compare September 13, 2021 14:46
@hinerm hinerm merged commit 78d902d into master Sep 13, 2021
@hinerm hinerm deleted the number-widget-format branch September 13, 2021 14:49
NicoKiaru added a commit to imagej/imagej.github.io that referenced this pull request Oct 19, 2021
Mention scijava-ui-swing improvement on number formatting done by @imagejan in scijava/scijava-ui-swing#54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No parameter attribute for number formats
5 participants