Bug 82641 - Currency drop-down ...
Summary: Currency drop-down ...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
4.3.0.2 rc
Hardware: Other All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: ToBeReviewed target:5.2.0
Keywords: difficultyInteresting, easyHack, skillCpp, topicCleanup
: 86344 (view as bug list)
Depends on:
Blocks: Split-Group-Buttons Calc-Toolbars
  Show dependency treegraph
 
Reported: 2014-08-15 01:05 UTC by Michael Meeks
Modified: 2017-10-22 20:24 UTC (History)
8 users (show)

See Also:
Crash report or crash signature:


Attachments
excel 2010 currency drop down (11.83 KB, image/png)
2014-12-07 19:55 UTC, Yousuf Philips (jay) (retired)
Details
This is done using the git log given in the bug description but the function CreatePopupWindow() is not changed (3.82 KB, patch)
2015-03-24 18:51 UTC, Rajat Vijay
Details
Implemnted the register function for my new class (5.52 KB, patch)
2015-03-30 17:22 UTC, Rajat Vijay
Details
Done with everything just final thing on uno command and confuguration file (6.49 KB, patch)
2015-04-04 07:06 UTC, Rajat Vijay
Details
Using xPopupMenu instead of xPopupMenuController in CreatePopupWindow (7.33 KB, patch)
2015-04-05 16:00 UTC, Rajat Vijay
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Meeks 2014-08-15 01:05:42 UTC
The calc toolbar for setting a currency format is pretty useful - but unfortuantely uses just one currency.

It'd be great to have a drop-down next to it with a list of currencies.

-Even-better- would be to have a list of the most commonly used (in fact the only ones ever used by that user would be even better - I only ~ever use USD, EUR, GBP myself) - in a "most frequently used" selection at the top - cf. the font drop-down.

The code that currently handles currency lists is in the format cells dialog (it'd be great to have the frequent currencies thing there too FWIW ;-)

cui/source/tabpages/numfmt.cxx

has that format page; and:

void SvxNumberFormatTabPage::FillCurrencyBox()

fills that currency goodness; m_pLbCurrency is 'member' 'pointer' 'Listbox' 'currency' when de-mangled ;-)

For an example of turning a toolbar button into a drop-down thingit; you could do worse than poke at:

git log -u -1 f60424b29622033ae02491bda328c304ae1bc05d

Replacing your work into the 'CreatePopupWindow' method I guess =)

Thanks !
Comment 1 Stefan Weiberg 2014-09-08 09:29:07 UTC
Hi Michael,

I'd like to implement this feature. Can I include the include/sfx2/tbxctrl.hxx to the cui/source/inc/numfmt.hxx as well? That way I could introduce a new method in tbxctrl.hxx where all these Toolboxes seem to be managed instead of fiddling around in other header files.

Thanks in advance for your help!

Cheers,
Stefan
Comment 2 Michael Meeks 2014-09-08 10:05:30 UTC
> Can I include the include/sfx2/tbxctrl.hxx to the cui/source/inc/numfmt.hxx
> as well ?

I suggest that you do whatever you need to do to make the most elegant, minimal, readable, maintainable patch possible =) Unless you are trying to include something that breaks layering (eg. including a VCL/ header into sal/ or something ;-) - then I wouldn't bother asking about that [ this direction is of course fine ]

In general, writing the feature once so it works, and then tweaking it afterwards based on review is a normal activity - so I'd implement something; push the patch to gerrit, and await feedback ;-) developers communicate in code & diffs mostly.
Comment 3 Stefan Weiberg 2014-09-08 11:27:56 UTC
Ok, thanks Michael. I will start with the implementation according to the git log -u -1 f60424b29622033ae02491bda328c304ae1bc05d changes.
Comment 4 Stefan Weiberg 2014-09-10 08:03:39 UTC
Hi Michael (and others ;)),

currently a very annoying problem is blocking my work on this feature request. I can't find the implementation of the existing currency button. I found the implementation of the click event (NumberFormatPropertyPanel.cxx) but I can't find the actual button to modify it.

Even with a git grep I couldn't find an implementation (maybe I am too blind to see it :D). Could you give me a hint where I can find this button?

Thanks a lot for your help in advance!

Cheers,
Stefan
Comment 5 Michael Meeks 2014-09-10 08:20:37 UTC
The currency button can be found via its UNO name:

cat ../../sc/uiconfig/scalc/toolbar/formatobjectbar.xml

which describes that toolbar; then:

git grep -3 NumberFormatCurrency

Then git grep for the SID_

However this mechanism is going to need to be replaced with a custom widget containing the drop-down so ... good to understand the old flow, but better to read:

git log -u -1 f60424b29622033ae02491bda328c304ae1bc05d

for how to do that.
Comment 6 Stefan Weiberg 2014-09-15 08:49:02 UTC
I have to admit that this bug is a bit too much for my C++ skills. I unfortunately have to drop my work on this bug.

Even after understanding all those changes in the commit f60424b29622033ae02491bda328c304ae1bc05d I couldn't think of a way to implement this dropdown menu for the currency button. My biggest problem was to find the place where I should implement it.

Therefore I unassign this bug and let it open for someone else.
Comment 7 Michael Meeks 2014-09-15 10:34:23 UTC
Hi Stefan - can you send me the file that you used to build notes on the code in, to the point that you gave up - I'd love to check that you're building a log of related code you've read, your assumptions etc.
Comment 8 Stefan Weiberg 2014-09-15 11:20:32 UTC
Hi Michael,

no problem at all. Here you go:

1. I read through the changes made in commit f60424b29622033ae02491bda328c304ae1bc05d. In my opinion it is a good way to implement a dropdown button for the currencies. (makes sense for me)

2. After that I read through the source code of NumberFormatPropertyPanel.cxx which implements the current Button. AFAIK this code implements only the handler for the button. (makes sense for me)

3. I searched the source code for hints on the code for the button itself (couldn't find the code). I used gdb to further understand the handling of the button click. Unfortunately it only showed me the handling of the mutexes. (don't know why it only shows the mutex handling)

4. After your hint on the signal/slot search and the ui interface I read some more code. This is where I stuck because of the concept of signal, slots and other stuff. (don't understand the implementation and dependencies of the current button)

I don't know where I should start with the implementation of the custom widget and how to get rid of the old button.

Greetings,
Stefan
Comment 9 Maxim Monastirsky 2014-09-15 13:16:45 UTC
Hi Stefan,

(In reply to comment #8)
> 2. After that I read through the source code of
> NumberFormatPropertyPanel.cxx which implements the current Button. AFAIK
> this code implements only the handler for the button. (makes sense for me)
No. This one is for the sidebar only.

> 3. I searched the source code for hints on the code for the button itself
> (couldn't find the code).
I guess there is no such code. Any "simple" toolbar button is handled by a generic controller, that simply executes the associated command. For a complex button, you need a specific controller that will manage it. What you need to do is to create a whole new class (which inherits from SfxToolBoxControl), and register it for the particular command by putting a call to YourClass::RegisterControl in the right place (f60424b29622033ae02491bda328c304ae1bc05d covers this too).

> 4. After your hint on the signal/slot search and the ui interface I read
> some more code. This is where I stuck because of the concept of signal,
> slots and other stuff. (don't understand the implementation and dependencies
> of the current button)
For the beginning you don't need to worry about that stuff. All that you need to know is that you're able to execute commands, and also get status notifications. Anyway [1] is a good source to understand how it works (although a bit outdated).

> I don't know where I should start with the implementation of the custom
> widget
A simple PopupMenu or ToolbarMenu should be enough I guess.

[1] https://wiki.openoffice.org/wiki/Framework/Article/Implementation_of_the_Dispatch_API_In_SFX2
Comment 10 Michael Meeks 2014-09-15 14:08:04 UTC
Hi there,

> 2. After that I read through the source code of NumberFormatPropertyPanel.cxx
> which implements the current Button. AFAIK this code implements only the
> handler for the button. (makes sense for me)

Ah ! in my logs, I would expect to see a file of a few thousand lines - with cut/paste portions of the text, and notes on call-stacks from the debugger when this file was hit, along with speculation on what each method does, and your assumptions listed etc. to form some sort of (not beautiful, not so readable - but very information dense) log of what you're thinking while code-reading that.

"I read some more code."

To me is a journal you wrote afterwards, not a dump of your scratch-pad of interesting bits of code you read along the way that may (or may not) be related. Again - you cannot possibly hope to work in this model:

until (understood)
{
    read-code   ----->  store in mind
}
... now I know what to do ...
... write code ...

This is simply going to fail; always. I am proposing you this model:

until (understood)
{
     read / edit existing notes
     until (sub-problems-understood)
     {
         read-code   ---> write notes
         if (too-complex)
         {
             break into new sub-problems : things I don't understand.
             assume certain sub-problems have various results:
                  -> write notes on the assumptions you made here
         } else // fully understood
             write notes on the sub-problem solution & remove from the list.
     } 
     ... go home, sleep, wake-up ...
}
... design a fix ...
... hack on this ...

> This is where I stuck because of the concept of signal, slots and other
> stuff. (don't understand the implementation and dependencies of the
> current button)

Ok - so we went around this loop once. This is good progress; now we know what we do not know - so - store the state of what you had learned up to now - and all those code pointers; and recurse - now you have a much easier problem: what is a signal / slot - how does that work ? what code can you read to understand and write notes on that ? ...

> I don't know where I should start with the implementation of the custom
> widget and how to get rid of the old button.

Ah - but you can't know everything; you can only learn some good techniques of systematically reducing problems to smaller ones, and then solving those. The smartest person in the world cannot keep all of this in their head, and put it together: so giving up on that is prolly a good step #1 ;-)
Comment 11 Stefan Weiberg 2014-09-16 09:28:17 UTC
Hi Michael and Maxim,

I will try to dig into this problem a bit deeper and to solve the problem. Feedback is the best motivation ;)

The presentation of Jan Holesovsky will be a big help for me:

https://speakerdeck.com/kendy/how-to-create-a-custom-widget

At least I want to implement the custom widget for the button.

Cheers and thx again,
Stefan
Comment 12 Stefan Weiberg 2014-09-16 16:11:44 UTC
Just to deliver a short update. The change in commit f60424b29622033ae02491bda328c304ae1bc05d used an already existent uielement from the file context in the menu bar (recentfilesmenucontroller.cxx in the framework module). Therefore I have to implement a similar dropdown menu for currencies. In addition I already implemented the button with the function calls for the recentFile context (just to get started). The note taking really helps a lot. 

Meanwhile my colleague could help me with gdb and how to find all the stuff in the backtrace. Right now I try to break down the backtrace and get to the constructor call of the RecentFilesToolBox to make my own function call on my newly implemented "button".
Comment 13 Michael Meeks 2014-09-17 10:39:54 UTC
Hi Stefan ! great news =) the RecentFiles drop-down seems like a good target indeed - sounds like you're getting your head around the problem nicely. If it were me - I would keep as many pieces stationary as possible, and iterate the code - so; first get the RecentFiles drop-down to override and kill that currency toolbar button ;-) then adapt it to show more etc. and git commit regularly so you can go back: it's easy to squash many changes later into a single pretty commit. Nice work & keep writing detailed / verbose notes =)
Comment 14 Mikeyy - L10n HR 2014-11-17 16:25:04 UTC
*** Bug 86344 has been marked as a duplicate of this bug. ***
Comment 15 Mikeyy - L10n HR 2014-11-17 16:35:53 UTC
Just an usability suggestion from my perspective.
Button should behave exactly like color background/font toolbar icons, meaning:
- when you press it, it should have DEFAULT behaviour
- default behaviour should be current currency icon behaviour (UI/keyboard language currency)
- but when you press arrow next to it, you should get dropdown with more choices
- choosing something else other then default currency, makes that selection DEFAULT until you close document

I hope you understand what I meant. :)
Comment 16 Michael Meeks 2014-11-17 21:27:15 UTC
completely agreed; we should also have a short-cut list of commonly used currencies in the drop-down ;-) Really hoping we can get this in for 4.4 - I would really like to use it myself =)
Comment 17 Yousuf Philips (jay) (retired) 2014-12-07 19:55:42 UTC
Created attachment 110542 [details]
excel 2010 currency drop down

I believe that doing a split button which list the most used currencies is the way to go, as more split buttons are coming to the toolbar.
Comment 18 Rishabh 2015-02-22 16:15:19 UTC
Any code pointers for assigning priority on the basis of most commonly used currency or I have to come up with my own algorithm ?
 
(In reply to Michael Meeks from comment #0)
> -Even-better- would be to have a list of the most commonly used (in fact the
> only ones ever used by that user would be even better - I only ~ever use
> USD, EUR, GBP myself) - in a "most frequently used" selection at the top -
> cf. the font drop-down.
Comment 19 Yousuf Philips (jay) (retired) 2015-02-22 20:33:17 UTC
I believe the list should be populated like so.

1) Default currency set in Tools > Options > Languages Settings > Language.
2) US Dollar
3) Euro
4) Japanese Yan
5) British Pound
6) Chinese Yuan
7) Swiss Franc
8) Currency of the user's system locale if its not in the list

I came up with this list and order by combining what Excel has and the most relevant entries in the lists below.

http://www.ibtimes.com/chinas-yuan-among-top-5-most-used-currencies-world-1797246
http://en.wikipedia.org/wiki/Template:Most_traded_currencies
http://www.investopedia.com/articles/forex/11/popular-currencies-and-why-theyre-traded.asp
Comment 20 Michael Meeks 2015-02-23 09:23:15 UTC
Sounds sensible to me =) thanks Jay - I guess http://www.xe.com/ has a common-currencies list based on their use-cases.

Having said that - it should be ~trivial to add the currencies used in the document to the list (assuming there are just a few of them - which is common), and of course any used 'recently' =) into some hybrid scoring thing.
Comment 21 Stefan Weiberg 2015-02-25 07:56:22 UTC
As I couldn't continue with my work on this bug I want at least to make my code changes available for others to use (if it is useful for anyone). I uploaded my changes as a draft to gerrit. If you want access to the draft just leave me a message.
Comment 22 Rishabh 2015-02-25 14:54:01 UTC
(In reply to Stefan Weiberg from comment #21)
> As I couldn't continue with my work on this bug I want at least to make my
> code changes available for others to use (if it is useful for anyone). I
> uploaded my changes as a draft to gerrit. If you want access to the draft
> just leave me a message.

Thanks a lot. I intend to work on it in a week and I will be grateful if I can get some head start :-) .
Comment 23 Stefan Weiberg 2015-02-25 15:21:17 UTC
(In reply to Rishabh from comment #22)
> (In reply to Stefan Weiberg from comment #21)
> > As I couldn't continue with my work on this bug I want at least to make my
> > code changes available for others to use (if it is useful for anyone). I
> > uploaded my changes as a draft to gerrit. If you want access to the draft
> > just leave me a message.
> 
> Thanks a lot. I intend to work on it in a week and I will be grateful if I
> can get some head start :-) .

I added you as a reviewer for the drafts. I hope that it helps you, even though it is rough around the edges and highly experimental (only a button with the recent files content). A lot of stuff is still missing and must be optimized but maybe you can gain some insight thanks to the draft.
Comment 24 Rajat Vijay 2015-03-22 18:48:25 UTC
Sir
I would like to work on this.
I have already started
+made the class SfxCurrencyListToolBoxControl:public SfxToolBoxControl
+it has a member function CreatePopupWindow 
+added SlotType = SfxStringItem in /sc/sdi/scalc.sdi --NumberFormatType since FillCurrencyBox is a function of NumberFormatType class
+added SfxCurrencyListToolBoxControl::RegisterControl(SID_NUMBER_TYPE_FORMAT, pMod) in /sc/source/ui/app/scdll.cxx --ScDLL::Init()
Comment 25 Michael Meeks 2015-03-23 09:22:21 UTC
Hi Rajat - great news =) please do feel free to ask questions by E-mail, but when you do - please include your patch (git diff) so far - it's always great to read & talk code. Looking forward to seeing this !
Comment 26 Rajat Vijay 2015-03-23 15:46:58 UTC
Tis is the error i am getting pls help--
/home/rajatvijay/libreoffice/workdir/CxxObject/sc/source/ui/app/scdll.o: In function `ScDLL::Init()':
scdll.cxx:(.text+0x192): undefined reference to `SfxCurrencyListToolBoxControl::RegisterControl(unsigned short, SfxModule*)'
collect2: error: ld returned 1 exit status
make[1]: *** [/home/rajatvijay/libreoffice/instdir/program/libsclo.so] Error 1
make: *** [build] Error 2

Also need some help in understanding and writing code for CreatePopupWindow()
for my new class , from the old one (as in class SfxRecentFilesToolBoxControl some names are difficult to understand)

Pls reply asap as i have to register for GSoC 2105

Thanks in advance!!!
Comment 27 Rajat Vijay 2015-03-23 15:56:24 UTC Comment hidden (obsolete)
Comment 28 Rajat Vijay 2015-03-23 16:24:26 UTC Comment hidden (obsolete)
Comment 29 Michael Meeks 2015-03-23 17:17:56 UTC
> Tis is the error i am getting pls help--

FYI - the -purpose- of asking you to contribute patches, is to determine competence and 'neediness' for mentoring. As I explained in comment #25
"please include your patch (git diff)" - it is not possible to debug code that we can't see. So - please do as asked. In addition, three duplicates of the same comment is unfortunate.

The error seems to suggest that the method 'RegisterControl' is not implemented, or not linked into the translation unit its referenced in ;-) did you implement that method ? and if so, did you include that object file into the makefiles.
Comment 30 Rajat Vijay 2015-03-24 18:51:45 UTC
Created attachment 114307 [details]
This is done using the git log given in the bug description but the function CreatePopupWindow() is not changed

still working in the function CreatePopupWindow() but has clue where to register my new class SfxCurrencyListToolBoxControl

please help !!!

Thanks in advance !!!
Comment 31 Michael Meeks 2015-03-25 10:09:31 UTC
> This is done using the git log given in the bug description but the function
> CreatePopupWindow() is not changed

Fair enough.

> still working in the function CreatePopupWindow() but has clue where
> to register my new class SfxCurrencyListToolBoxControl

The call to the registration is fine; the call here:

+SFX_IMPL_TOOLBOX_CONTROL(SfxCurrencyListToolBoxControl, SfxStringItem);

Should implement that Register method for your new class cf:

include/sfx2/tbxctrl.hxx-#define SFX_IMPL_TOOLBOX_CONTROL(Class, nItemClass) \
Comment 32 Rajat Vijay 2015-03-30 17:22:03 UTC
Created attachment 114469 [details]
Implemnted the register function for my new class

Implemented register function for my new class: SfxCurrencyListToolBoxControl
Having diffculty in Adding list of currencies in my CreatePopupWindow() function for my new class !!!
Only work left to do is in CreatePopupWindow() ?

Thanks in advanace !!!
Comment 33 Rajat Vijay 2015-04-04 07:06:16 UTC
Created attachment 114607 [details]
Done with everything just final thing on uno command and confuguration file

Cant find the uno command url and the respective configuration file for the list of currencies in format cell dialog box NumberFormatCurrency

Reply asap

Thanks in advance !
Comment 34 Rajat Vijay 2015-04-05 16:00:47 UTC
Created attachment 114634 [details]
Using xPopupMenu instead of xPopupMenuController in CreatePopupWindow

make command giving the error "undefied reference to SvxNumberFormatShell::GetCurrencySymbols( std::vector< OUString >, sal_uInt16 )"

Help needed ASAP 
 
Thanks in advance !
Comment 35 Robinson Tryon (qubit) 2015-12-14 05:01:15 UTC Comment hidden (obsolete)
Comment 36 Robinson Tryon (qubit) 2016-02-18 14:51:40 UTC Comment hidden (obsolete)
Comment 37 Mohammed Abdul Azeem 2016-03-02 21:06:50 UTC
I'm working on this.
I could convert the button to drop down split button, now I'm trying to populate the list as suggested. :-)
Comment 38 jani 2016-03-02 21:25:39 UTC
(In reply to Mohammed Abdul Azeem from comment #37)
> I'm working on this.
> I could convert the button to drop down split button, now I'm trying to
> populate the list as suggested. :-)

Please assign the bug to you, so we can see you work on it.

rgds
jan i
Comment 39 Commit Notification 2016-03-19 14:21:36 UTC
Mohammed Abdul Azeem committed a patch related to this issue.
It has been pushed to "master":

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

tdf#82641 Currency drop-down list

It will be available in 5.2.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 40 Michael Meeks 2016-03-19 20:38:48 UTC
Thanks to Mohammed for fixing this one =)
Comment 41 Yousuf Philips (jay) (retired) 2016-03-20 10:45:59 UTC
Looks good Mohammed. Is there a means where we can align the 3 pieces of data into columns, so its easier to see?

Version: 5.2.0.0.alpha0+
Build ID: 157c60a7077098bb9b142e825fee8bec18d015db
CPU Threads: 2; OS Version: Linux 4.2; UI Render: default; 
TinderBox: Linux-rpm_deb-x86@71-TDF, Branch:master, Time: 2016-03-20_00:57:35
Locale: en-US (en_US.UTF-8)
Comment 42 Mohammed Abdul Azeem 2016-03-20 18:04:46 UTC
Jay, that's a nice idea. Even I felt the Entries were little too long. Or we can make two list boxes in the same pop-up window (currency and country-lang), selecting a currency would fill up associated country-language in the other list box? Just a thought.  I would love to try that out. I'm occupied for a week or so. I would look into it whenever possible. :)
Comment 43 Commit Notification 2016-03-23 13:44:15 UTC
Mohammed Abdul Azeem committed a patch related to this issue.
It has been pushed to "master":

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

tdf#82641 Changes to currency drop-down list

It will be available in 5.2.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 44 Mikeyy - L10n HR 2016-07-04 08:30:18 UTC
Thank you for fix.
Not sure if I should make suggestion here or open another bug?

List of available choices is very long. Can we have some way to prioritize choices based on locale?
Every locale has their local i18n file, and list could be defined there.

For example, for my locale (HR) first choices are Arabian, Armenian etc., which are used very little.
I would like to arrange list so first 5 choices can be DEFAULT KN (HR currency), EUR Germany standard, Dollar USA, Pound, Yen, Yuan. Rest would go in alphabetical order.

Just a suggestion. :)
Comment 45 Michael Meeks 2016-07-04 08:40:51 UTC
Capturing the last 5 used currencies and remembering them would solve the problem; IIRC there is another bug open for that somewhere, but its not making progress - Gulsah was working on it but (IIRC) gave up.

HTH.