bug in qx.ui.table.cellrenderer.Number #9340
Comments
|
Hi @dvoral, thanks for the bug report - could you try implementing a Pull Request with this change? That's the fastest way to get a fix integrated into the core |
|
Sigh. There is no good solution to this problem. On the one hand, we can change the We could also change qx.ui.table.cellrenderer.Number so that it does what its name implies and ensures that every field contains a number. This would mean that if the data is null or an empty string, we put a 0 in the cell. That way, the cell editor (which was the basis of the OP's problem for which this issue was created) would always get a valid initial value, and not display NaN. That is also not backbackward compatible, because programs may well expect the ability to have empty fields in a Number column. I'm open to ideas. Derrell |
|
A number column in a table used for data entry MUST allow empty cells.
How about a separat cellrenderer class? That would not break anything.
Cheers,
Fritz
…Sent from my iPhone
On 25 May 2017, at 21:20, Derrell Lipman ***@***.***> wrote:
Sigh. There is no good solution to this problem.
On the one hand, we can change the format method of qx.util.format.NumberFormat so that an empty string is not NaN, but that is not backward compatible with apps that expect an empty string to return NaN.
We could also change qx.ui.table.cellrenderer.Number so that it does what its name implies and ensures that every field contains a number. This would mean that if the data is null or an empty string, we put a 0 in the cell. That way, the cell editor (which was the basis of the OP's problem for which this issue was created) would always get a valid initial value, and not display NaN. That is also not backbackward compatible, because programs may well expect the ability to have empty fields in a Number column.
I'm open to ideas.
Derrell
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
@dvoral did you find a solution for this bug? Please share you're solution/thoughts. Thank you. |
|
Yes I did. I wrote on the forum what I did. On June 4 I wrote:
"I managed to fix the NaN while editing problem. I wrote an override of the
setValidationFunction of the qx.ui.table.celleditor.TextField."
Nobody asked until now what I did.
My solution:
var intEditor = new qx.ui.table.celleditor.TextField();
intEditor.setValidationFunction (
function( value ) {
if (value == "") {
value = null;
}
if (value != null) {
value = parseFloat(value);
}
return value;
});
Hope this is helpful.
…On Tue, Jun 13, 2017 at 9:22 AM, Dietrich Streifert < ***@***.***> wrote:
@dvoral <https://github.com/dvoral> did you find a solution for this bug?
Please share you're solution/thoughts. Thank you.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#9340 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/Abcqy2I-MeQwIj-bG6LxLmVcvkRVeaUEks5sDiqRgaJpZM4NiDpg>
.
|
|
@dvoral would you please be so kind to create a pull request for |
|
Can you please send me a link where I do that?
And instructions if it's not clear for a first-time user. :)
…On Tue, Jun 13, 2017 at 10:57 AM, Dietrich Streifert < ***@***.***> wrote:
@dvoral <https://github.com/dvoral> would you please be so kind to create
a pull request for qx.ui.table.celleditor.TextField. that would be great!
thank you!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#9340 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/Abcqy5_LRKa2PeFhYoVFJbAZVwzI17tAks5sDkDcgaJpZM4NiDpg>
.
|
|
I'd suggest adding this as a static function |
|
@dvoral there are several tutorials out there but here a short version which works completely in the browser:
That's it. Try it! You could do no harm to the original repo. We won't let you :-) |
|
Thanks for your detailed instructions. I've made the pull request. |
|
Hi,
Thank you for your email - please note that I am on annual leave, returning on June 21st
If you have an emergency support issue, please describe your issue in detail in email and then send a text message to the mobile number on your invoice.
Regards
John Spackman
|
|
@dvoral you created the PR from dvoral/qooxdoo:dvoral-fix-9340 against your own fork master dvoral/qooxdoo:master Please navigate again to https://github.com/dvoral/qooxdoo/pulls to the PR list and then click the green button "New pull request". There will appear four buttons for selection on the "Comparing changes" page. Please select there from the rightmost button (Text beginning with "compare:") the dvoral-fix-9340 branch. This way you compare your branch against qooxdoo/qooxdoo:master. You will then be offered again the "Create pull request" green button which allows you to publish your PR in https://github.com/qooxdoo/qooxdoo/pulls |
|
Hope I did it correctly this time.
…On Wed, Jun 14, 2017 at 12:24 PM, Dietrich Streifert < ***@***.***> wrote:
@dvoral <https://github.com/dvoral> you create the PR from
dvoral/qooxdoo:dvoral-fix-9340 against your own forks master
dvoral/qooxdoo:master
Please navigate again to https://github.com/dvoral/qooxdoo/pulls to the
PR list and then click the green button "New pull request".
There will appear four buttons for selection on the "Comparing changes"
page.
Please select there from the rightmost button (Text beginning with
"compare:") the dovarl-fix-9340 branch. This way you compare your branch
against qooxdoo/qooxdoo:master.
You will then be offered again the "Create pull request" green button
which allows you to publish your PR in https://github.com/qooxdoo/
qooxdoo/pulls
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#9340 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/Abcqy_3JpfQ8AySnfwurEI7s3XO1m0Cmks5sD6bqgaJpZM4NiDpg>
.
|
|
@dvoral if you need to add changes to the PR or change your current PR, you simply navigate to the corresponding branch https://github.com/dvoral/qooxdoo/tree/dvoral-fix-9340 and edit what you want and save to the same branch (here dvoral-fix-9340). As soon as you've done that, the changes appear in the PR #9348 |
|
I changed the place of the check on null string as recommended.
…On Wed, Jun 14, 2017 at 3:28 PM, Dietrich Streifert < ***@***.***> wrote:
@dvoral <https://github.com/dvoral> if you need to add changes to the PR
or change your current PR, you simply navigate to the corresponding branch
https://github.com/dvoral/qooxdoo/tree/dvoral-fix-9340 and edit what you
want and save to the same branch (here dvoral-fix-9340).
As soon as you've done that, the changes appear in the PR #9348
<#9348>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#9340 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/Abcqy3AcH87zAuKiXEFbVRni_phjbH-Cks5sD9IBgaJpZM4NiDpg>
.
|
|
Fixed via #9348 |
If the cell is empty (null) and is being edited, then NaN displays.
From Derrell Lipman:
if (cellInfo.value || cellInfo.value == 0) {
should be
if (cellInfo.value || cellInfo.value === 0) {
Note the triple '=' so that it catches the number 0 but not something that's falsy like null.
We should also change the else condition where it says cellInfo.value == 0.
The text was updated successfully, but these errors were encountered: