Skip to content

Refactor/tweak BitcoinGUI#3763

Merged
UdjinM6 merged 3 commits into
dashpay:developfrom
UdjinM6:refactor_BitcoinGUI
Nov 4, 2020
Merged

Refactor/tweak BitcoinGUI#3763
UdjinM6 merged 3 commits into
dashpay:developfrom
UdjinM6:refactor_BitcoinGUI

Conversation

@UdjinM6
Copy link
Copy Markdown

@UdjinM6 UdjinM6 commented Oct 5, 2020

Fixing confusing names and doing some code flow cleanup. Please see individual commits.

Based on #3762 atm

@UdjinM6 UdjinM6 added this to the 17 milestone Oct 5, 2020
@UdjinM6 UdjinM6 marked this pull request as draft October 5, 2020 16:39
@UdjinM6 UdjinM6 force-pushed the refactor_BitcoinGUI branch from 767db72 to 5627203 Compare October 28, 2020 00:42
@UdjinM6 UdjinM6 marked this pull request as ready for review October 28, 2020 15:43
@PastaPastaPasta
Copy link
Copy Markdown
Member

Won't this cause EVEN MORE conflicts in the future?

@UdjinM6
Copy link
Copy Markdown
Author

UdjinM6 commented Oct 30, 2020

@PastaPastaPasta Theoretically - yes. But the only place with potential merge conflicts I see when looking at git diff 0.17..master -- src/qt/bitcoingui.cpp is

-    connect(overviewAction, SIGNAL(triggered()), this, SLOT(showNormalIfMinimized()));
-    connect(overviewAction, SIGNAL(triggered()), this, SLOT(gotoOverviewPage()));
-    connect(sendCoinsAction, SIGNAL(triggered()), this, SLOT(showNormalIfMinimized()));
-    connect(sendCoinsAction, SIGNAL(triggered()), this, SLOT(gotoSendCoinsPage()));
-    connect(sendCoinsMenuAction, SIGNAL(triggered()), this, SLOT(showNormalIfMinimized()));
-    connect(sendCoinsMenuAction, SIGNAL(triggered()), this, SLOT(gotoSendCoinsPage()));
-    connect(receiveCoinsAction, SIGNAL(triggered()), this, SLOT(showNormalIfMinimized()));
-    connect(receiveCoinsAction, SIGNAL(triggered()), this, SLOT(gotoReceiveCoinsPage()));
-    connect(receiveCoinsMenuAction, SIGNAL(triggered()), this, SLOT(showNormalIfMinimized()));
-    connect(receiveCoinsMenuAction, SIGNAL(triggered()), this, SLOT(gotoReceiveCoinsPage()));
-    connect(historyAction, SIGNAL(triggered()), this, SLOT(showNormalIfMinimized()));
-    connect(historyAction, SIGNAL(triggered()), this, SLOT(gotoHistoryPage()));
+    connect(overviewAction, &QAction::triggered, [this]{ showNormalIfMinimized(); });
+    connect(overviewAction, &QAction::triggered, this, &BitcoinGUI::gotoOverviewPage);
+    connect(sendCoinsAction, &QAction::triggered, [this]{ showNormalIfMinimized(); });
+    connect(sendCoinsAction, &QAction::triggered, [this]{ gotoSendCoinsPage(); });
+    connect(sendCoinsMenuAction, &QAction::triggered, [this]{ showNormalIfMinimized(); });
+    connect(sendCoinsMenuAction, &QAction::triggered, [this]{ gotoSendCoinsPage(); });
+    connect(receiveCoinsAction, &QAction::triggered, [this]{ showNormalIfMinimized(); });
+    connect(receiveCoinsAction, &QAction::triggered, this, &BitcoinGUI::gotoReceiveCoinsPage);
+    connect(receiveCoinsMenuAction, &QAction::triggered, [this]{ showNormalIfMinimized(); });
+    connect(receiveCoinsMenuAction, &QAction::triggered, this, &BitcoinGUI::gotoReceiveCoinsPage);
+    connect(historyAction, &QAction::triggered, [this]{ showNormalIfMinimized(); });
+    connect(historyAction, &QAction::triggered, this, &BitcoinGUI::gotoHistoryPage);

which should be trivial to resolve imo.

Copy link
Copy Markdown
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

Hmmm... okay... utACK

Copy link
Copy Markdown

@xdustinface xdustinface left a comment

Choose a reason for hiding this comment

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

ACK

@UdjinM6 UdjinM6 merged commit c7746f6 into dashpay:develop Nov 4, 2020
@UdjinM6 UdjinM6 deleted the refactor_BitcoinGUI branch November 26, 2020 13:28
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Mar 12, 2022
* Rename QToolButton-s

* Move buttons from createActions() to createToolBars()

Also, drop `showNormalIfMinimized()` connect for buttons cause it makes no sense

* Tweak `BitcoinGUI` to let `updateToolBarShortcuts()` work with `QButtonGroup` only
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.

3 participants