Skip to content

Using the style to format number in the swing UI. #52

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

Conversation

BoudewijnvanLangerak
Copy link
Contributor

Using the style to format number in the swing UI.

Copy link
Member

@imagejan imagejan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot, @BoudewijnvanLangerak, for your suggestions. In my opinion, it makes a lot of sense to allow setting the format of numeric inputs via the style parameter as you propose.

If @ctrueden agrees in principle to what I've done in scijava/scijava-common#384 so far, I'd like to include the proposed change of this pull request into that other PR.

Comment on lines +143 to +147
for (final String s : widgetStyle.split(",")) {
if (s.startsWith("format:")) {
format = s.substring(s.indexOf(":") + 1).trim();
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With scijava/scijava-common#384, I tried to centralize parsing of the style attribute into a WidgetStyle class (as proposed by @ctrueden) and to fix some related issues, such as inconsistent handling of spaces with style definitions.

To keep the code base as DRY as possible, I suggest relying on that WidgetStyle class instead of this getFormat implementation. The latter would be specific to scijava-ui-swing whereas the former would allow usage from any UI implementation.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@imagejan , basically that does sound like a good idea, but that PR has been open since April. We would rather have this be merged and available sometime soon and when your widget style becomes available, start using it as of then.

We are however not sure what the release cycle is. Is there a chance we can get this code released earlier if we keep it contained to just ui-swing?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I merged scijava/scijava-common#384 and released scijava-common 2.85.0. @sunsear @BoudewijnvanLangerak Could you please add <scijava-common.version>2.85.0</scijava-common.version> to the POM on this branch, and try to implement @imagejan's suggestions? I'll then try to get this PR merged quickly. I'm sorry I'm so slow merging PRs—I maintain too many things, and so the release cycle of everything suffers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With scijava/scijava-common#405, it should be possible to replace this getFormat() method by a call to:

WidgetStyle.getStyleModifier(model.getItem().getWidgetStyle(), "format")

imagejan added a commit to scijava/scijava-common that referenced this pull request Oct 30, 2020
While getStyleModifiers() is useful for things like "extensions:png/gif/bmp", this commit adds a new getStyleModifier() method that simply returns the suffix after the colon, for cases like "format:#0.001".

See also scijava/scijava-ui-swing#52.
imagejan added a commit to scijava/scijava-common that referenced this pull request Oct 30, 2020
While getStyleModifiers() is useful for things like "extensions:png/gif/bmp", this commit adds a new getStyleModifier() method that simply returns the suffix after the colon, for cases like "format:#0.001".

See also scijava/scijava-ui-swing#52.
Copy link
Member

@imagejan imagejan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just added a few comments. It would be nice if the behavior in absence of any format style declaration could be improved as well.

Comment on lines +143 to +147
for (final String s : widgetStyle.split(",")) {
if (s.startsWith("format:")) {
format = s.substring(s.indexOf(":") + 1).trim();
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With scijava/scijava-common#405, it should be possible to replace this getFormat() method by a call to:

WidgetStyle.getStyleModifier(model.getItem().getWidgetStyle(), "format")

@@ -119,6 +119,11 @@ else if (model.isStyle(NumberWidget.SLIDER_STYLE)) {
final SpinnerNumberModel spinnerModel =
new SpinnerNumberModelFactory().createModel(value, min, max, stepSize);
spinner = new JSpinner(spinnerModel);
String format = getFormat(model);
if (format != null) {
spinner.setEditor(new JSpinner.NumberEditor(spinner, format));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great to have some sensible guessing on the format (based on min, max and/or default value) in case format is null here.

imagejan added a commit to scijava/scijava-common that referenced this pull request Dec 18, 2020
While getStyleModifiers() is useful for things like "extensions:png/gif/bmp", this commit adds a new getStyleModifier() method that simply returns the suffix after the colon, for cases like "format:#0.001".

See also scijava/scijava-ui-swing#52.
imagejan added a commit to scijava/scijava-common that referenced this pull request Jan 19, 2021
While getStyleModifiers() is useful for things like "extensions:png/gif/bmp", this commit adds a new getStyleModifier() method that simply returns the suffix after the colon, for cases like "format:#0.001".

See also scijava/scijava-ui-swing#52.
@imagejan
Copy link
Member

I'm now closing this PR in favor of #54 that includes these changes.

@imagejan imagejan closed this Jan 20, 2021
hinerm pushed a commit to scijava/scijava-common that referenced this pull request Sep 7, 2021
While getStyleModifiers() is useful for things like "extensions:png/gif/bmp", this commit adds a new getStyleModifier() method that simply returns the suffix after the colon, for cases like "format:#0.001".

See also scijava/scijava-ui-swing#52.
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.

4 participants