Removing spin buttons

[Aniket21mathur]

The spin buttons were not required as per the issue.

Instead of using spinButton used simple text entry with checks for integer inputs.

Fixes https://github.com/sugarlabs/convert/issues/14

[Aniket21mathur]

@quozl I used simple Gtk.Entry() in place of Gtk.SpinButton().So accordingly I made few changes based on few observations: 1)The input number or the number on the left hand side always had one decimal place so I fixed it to "1" or the decimal value of ".0". 2)There need to be a check for non integer entries so I used try and catch and always reset the value to 1.0 whenever a string is entered. Thanks! Please review.

[quozl]

Thanks. Reviewed. Comments;

• [x] is self.adjustment required any longer? You removed the last use of the variable.
• [x] rather than accept any key, please use Gtk.Editable.signals.insert_text to detect and ignore text that is not numeric? I've checked; we use insert-text in a few other activities, but none of them stop the signal emission as the documentation suggests.

[Aniket21mathur]

Agreed, made the suggested changes. Thanks :smile:

[quozl]

Thanks. Good progress. Reviewed. Comments;

• [ ] the regular expression match does not accept a decimal point, negative, or commas; is there a better function for numeric input validation you can use? Or place a float() call inside an exception handler in the insert-text callback,
• [x] self.spin is no longer a good name for the variable that references a Gtk.Entry widget, can you think of a better name?
• [x] self.spin is used elsewhere, see the set_value, perhaps it should change,
• [x] our callback function name convention is to suffix with _cb, and prefix with _ when the callback won't be used by any other code, but the prevailing style in this activity is underscore without _cb suffix; please either use the prevailing style or fix all the callback function names in a separate patch and place them before your patches here,
• [x] the connect call is on two lines, but it need only be on one line,
• [x] the commit messages no longer briefly says what is done and why; perhaps a rewrite of commit message may help.

[Aniket21mathur]

Thanks for reviewing, made the above changes. :smile:

[quozl]

Thanks. Nearly done. Reviewed. Comments on the user experience of value entry;

• [ ] numbers with commas are accepted but not processed; e.g. logs contain ValueError: invalid literal for float(): 1,000,
• [ ] the sign can be positioned wrongly, but not processed; e.g. logs contain ValueError: invalid literal for float(): 12-45,
• [ ] the test seems to be written for single characters; when multiple are entered, such as with a middle-mouse-button click using the primary selection, or a ctrl+v, the validation fails to prevent the invalid entry, see screenshot;

[Aniket21mathur]

Thanks for reviewing worked on the suggestions you made and pushed the changes :smile: .

[quozl]

It's getting complicated, isn't it? For some locales, a comma is a thousands separator, and for others it is a decimal point. Might it be better to place a call to float() inside an exception handler?

[Aniket21mathur]

It's getting complicated, isn't it? For some locales, a comma is a thousands separator, and for others it is a decimal point.

Now there is exactly one function where a comma is treated as a decimal point

def _flip(self, widget): active_combo1 = self.combo1.get_active() active_combo2 = self.combo2.get_active() self.combo1.set_active(active_combo2) self.combo2.set_active(active_combo1) value = float(self.label.get_text().split(' ~ ')[1].replace(',', '.')) self.value_entry.set_text(str(value)) self.call()

And after the above changes there is no longer need of this line

.replace(',', '.'))

to calculate value for _flip function as "," will already be handled by _update_label function.So there is no concept of commas as decimals in the code now. Thanks!

[quozl]

Thanks. Tested.

• "1,5" was accepted as "15.00",
• a copy and paste of "12+45" no longer pasted any characters,
• pressing enter after the number was ignored; the convert button had to be pressed, or tab followed by enter; but it was always thus.