Bug 86119 - TOOLBAR: Number Format Decimal and Standard doesnt show toggle correctly
Summary: TOOLBAR: Number Format Decimal and Standard doesnt show toggle correctly
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: Other All
: medium normal
Assignee: Gülşah Köse
URL:
Whiteboard: target:5.4.0 target:5.5.0 target:5.4.0.1
Keywords:
Depends on:
Blocks: Number-Format Calc-Toolbars
  Show dependency treegraph
 
Reported: 2014-11-10 16:02 UTC by Yousuf Philips (jay) (retired)
Modified: 2017-10-22 20:24 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Yousuf Philips (jay) (retired) 2014-11-10 16:02:13 UTC
The 'Number Format: Decimal' button (.uno:NumberFormatDecimal) doesnt function like the other buttons in that when you click it, it doesnt show the toggle in state.

Version: 4.4.0.0.alpha1+
Build ID: d59b9b4af36148e4d8df8f3e3492116d378642e2
TinderBox: Linux-rpm_deb-x86@45-TDF, Branch:master, Time: 2014-11-06_00:36:43
Comment 1 Buovjaga 2014-11-16 19:02:50 UTC
Reproduced.

Win 7 64-bit Version: 4.4.0.0.alpha2+
Build ID: b021b5983c62e266b82d9f0c5c6d8d8900553827
TinderBox: Win-x86@39, Branch:master, Time: 2014-11-12_01:10:08
Comment 2 Yousuf Philips (jay) (retired) 2014-11-19 01:05:27 UTC
The same problem happens with Standard button (.uno:NumberFormatStandard) as well.
Comment 3 Yousuf Philips (jay) (retired) 2014-11-19 23:35:51 UTC
CCing Samuel and Maxim for assistance in fixing this button clicking state issue as its front and center in calc.
Comment 4 Maxim Monastirsky 2014-11-20 08:10:16 UTC
The problem here is not with the state update of the button, but with the missing functionality for the "off" state. Right not it's only one way button.

Regarding .uno:NumberFormatStandard I don't think it's a bug. To make a toggle button we need two possible states - on and off. Any other command has these two possibilities. For example - you can format a number as precent, or you can reset the display to show the raw value of the cell. But .uno:NumberFormatStandard just resets whatever special format you have, to show the raw value instead. So there is no second state.
Comment 5 Yousuf Philips (jay) (retired) 2014-11-20 14:22:51 UTC
(In reply to Maxim Monastirsky from comment #4)
> The problem here is not with the state update of the button, but with the
> missing functionality for the "off" state. Right not it's only one way
> button.

Thanks for tracking down the culprit.

> Regarding .uno:NumberFormatStandard I don't think it's a bug. To make a
> toggle button we need two possible states - on and off. Any other command
> has these two possibilities. For example - you can format a number as
> precent, or you can reset the display to show the raw value of the cell. But
> .uno: just resets whatever special format you have, to
> show the raw value instead. So there is no second state.

Isnt when a cell is in its raw value state equivalent to a toggled .uno:NumberFormatStandard state and when a cell isnt in its raw value state a non-toggled state. That is how i consider all of the buttons. :D
Comment 6 Maxim Monastirsky 2014-11-20 15:34:20 UTC
(In reply to Jay Philips from comment #5)
> Isnt when a cell is in its raw value state equivalent to a toggled
> .uno:NumberFormatStandard state and when a cell isnt in its raw value state
> a non-toggled state. That is how i consider all of the buttons. :D
With such interpretation, when the button is already pressed, clicking on it would have no effect, unless you click on another formatting button instead. That's fine for me, although a bit different behavior than the other buttons.
Comment 7 Yousuf Philips (jay) (retired) 2014-11-21 07:39:08 UTC
(In reply to Maxim Monastirsky from comment #6)
> With such interpretation, when the button is already pressed, clicking on it
> would have no effect, unless you click on another formatting button instead.

Yes i had seen that behaviour of clicking it and then it resetting it back to general/standard. If it was possible to fit all the different number format buttons into the toolbar, then it would be better that they all acted like radio buttons similar to text alignment, but as thats not possible, clicking the button to deactivate that number format has a benefit here.
Comment 8 QA Administrators 2015-12-20 16:18:44 UTC Comment hidden (obsolete)
Comment 9 QA Administrators 2017-01-03 19:50:13 UTC Comment hidden (obsolete)
Comment 10 Gülşah Köse 2017-03-21 20:31:06 UTC
Needs code review : https://gerrit.libreoffice.org/#/c/35518/
Comment 11 Commit Notification 2017-03-24 08:00:31 UTC
Gulsah Kose committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=63a5e37a45e48b9f0c9ae2b632042e2a2d57dcbf

tdf#86119 Fix toggle behaviour of SID_NUMBER_TWODEC.

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 12 Yousuf Philips (jay) (retired) 2017-04-06 16:03:48 UTC
@Gulsah: I can confirm that number format decimal now works correctly, but no fix was done to number format standard. See comment 6 and comment 7.

Version: 5.4.0.0.alpha0+
Build ID: 0c551c86f701c1f1865760e122d901c933b8fbe5
CPU threads: 2; OS: Linux 3.19; UI render: default; VCL: gtk2; 
TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:master, Time: 2017-04-06_00:40:42
Locale: en-US (en_US.UTF-8); Calc: group
Comment 13 Gülşah Köse 2017-04-20 07:20:23 UTC
@Yousouf that problem is more complicated and not restricted with decimal and standart. It should be seperate bug i think.

Here is a table which attributes can work together:(but not work together as toggle)

	        Can work together
Standard	Percent, Currency
Currency	Decimal
Percent	        Standard,Percent, Currency, Scientific
Decimal	        Currency, Percent
Date	
Scientific	Percent

That different radio button toggle behaviour should be set from scratch and i think a can do it. So that bug can be closed i think. What do you think?
Comment 14 Gülşah Köse 2017-04-20 07:27:40 UTC
I mean "make standard button toggle" work is wasted effort because then it will be overwritten.
Comment 15 Yousuf Philips (jay) (retired) 2017-04-20 12:22:49 UTC
(In reply to Gülşah Köse from comment #13)
> That different radio button toggle behaviour should be set from scratch and
> i think a can do it. So that bug can be closed i think. What do you think?

Its fine with me if you want to work on a revised solution in another bug. Please CC this bug when you create that one. :D
Comment 16 Gülşah Köse 2017-05-29 13:14:05 UTC
@Yousuf, I was wrong about this. Each attribute should work alone for one cell. so, I am going to fix the Standart/General button's behaviour now.

To be sure: general button should be default on(pressed) and all of the other attributes should change general button's state to off(unpressed), when all of the other attributes off(by clicking on them), general button should be on again? Could you confirm that?
Comment 17 Yousuf Philips (jay) (retired) 2017-05-29 23:32:38 UTC
(In reply to Gülşah Köse from comment #16)
> To be sure: general button should be default on(pressed) and all of the
> other attributes should change general button's state to off(unpressed),
> when all of the other attributes off(by clicking on them), general button
> should be on again? Could you confirm that?

Yep that's right. You can see this same behaviour in the drop down listbox in Number Format content panel of the Properties tab in the Sidebar.
Comment 18 Commit Notification 2017-05-30 17:42:40 UTC
Gulsah Kose committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=82beda528fb74ae1c6b5e12a98dbf5d4e9e4c0db

tdf#86119 Fix toggle behaviour of SID_NUMBER_STANDARD.

It will be available in 5.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 19 Commit Notification 2017-06-01 14:31:46 UTC
Gulsah Kose committed a patch related to this issue.
It has been pushed to "libreoffice-5-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=9c7e9cbdaf06b0bbd1f8a476e77b9a3172c99ef6&h=libreoffice-5-4

tdf#86119 Fix toggle behaviour of SID_NUMBER_STANDARD.

It will be available in 5.4.0.1.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 20 Commit Notification 2017-06-07 15:02:25 UTC
Gulsah Kose committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=864f3d6d305240823304f8bad7b5d2603bbc0339

tdf#86119 Generalize number format control.

It will be available in 5.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 21 Commit Notification 2017-06-07 19:55:43 UTC
Gulsah Kose committed a patch related to this issue.
It has been pushed to "libreoffice-5-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=07dcac8a0057fc24e8cf0a8daebacd97da9c982c&h=libreoffice-5-4

tdf#86119 Generalize number format control.

It will be available in 5.4.0.1.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.