gui: Show current wallet name in window title by promag · Pull Request #15149 · bitcoin/bitcoin

promag · 2019-01-12T00:00:00Z

src/qt/bitcoingui.cpp

src/qt/bitcoingui.h

jonasschnelli · 2019-01-12T00:14:06Z

Yeah. Why not.
Concept ACK.
I suggest removing the text part "Wallet" | "Node".

promag · 2019-01-12T00:19:24Z

I suggest removing the text part "Wallet" | "Node".

@jonasschnelli I kind of agree, not very helpful I guess.

DrahtBot · 2019-01-12T01:05:56Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#15153 (gui: Add Open Wallet menu by promag)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

promag · 2019-01-12T03:32:23Z

Now with Jonas suggestion.

benthecarman · 2019-01-12T06:11:59Z

tACK 530ec6c

hebasto · 2019-01-13T14:56:16Z

Concept ACK.

src/qt/bitcoingui.cpp

src/qt/bitcoingui.h

luke-jr · 2019-01-13T18:16:13Z

~~IMO this doesn't really make sense, since the same window is used for all loaded wallets currently.~~

promag · 2019-01-13T18:23:31Z

@luke-jr chrome shows the current tab title. code editors show current file. lots of other examples..

jonasschnelli · 2019-01-13T19:16:42Z

IMO this doesn't really make sense, since the same window is used for all loaded wallets currently.

IMHO, window titles are okay to be context sensitive.

hebasto · 2019-01-14T08:01:11Z

@promag from IRC

walletFrame can be empty right?

Exactly! That is why I suppose checking walletFrame is sufficient and checking enableWallet and walletView is redundant.

promag · 2019-01-14T08:14:16Z

@hebasto dropped enableWallet check, walletFrame is indeed sufficient:

bitcoin/src/qt/bitcoingui.cpp

Lines 101 to 104 in a9b71a0

    
           if(enableWallet) 
        
           { 
        
               /** Create wallet frame and make it the central widget */ 
        
               walletFrame = new WalletFrame(_platformStyle, this);

Still checking walletView for the case the last wallet is unloaded.

Let me know so I can squash.

hebasto · 2019-01-14T08:42:12Z

tACK c1e8f74f326f862adbf5a326eb1b9a65e6eb1edd

Checked the case when all wallets are unloaded:

promag · 2019-01-14T08:52:26Z

Thanks @hebasto.

laanwj · 2019-01-14T13:39:49Z

Concept ACK, agree it's fine for window titles to be dynamic and context sensitive.

hebasto · 2019-01-14T22:38:03Z

re-tACK b0b2d112550817e31e469a19a2d69bcef97ae4be

Sjors · 2019-01-15T12:08:37Z

Concept ACK, code changes look fine, tested.

I don't like the default behavior:

"- [default wallet]" is visual clutter. If there's only one wallet loaded and it doesn't have a custom name, I would just show nothing.

promag · 2019-01-15T14:29:40Z

If there's only one wallet loaded and it doesn't have a custom name, I would just show nothing

That would be the same as no wallet is loaded. Beside, we already use that wording for the default wallet name. And this won't be an issue if the goal is to avoid creating the default wallet.

promag · 2019-01-15T14:36:15Z

Rebased after #14594 merge.

Sjors · 2019-01-15T15:35:48Z

That would be the same as no wallet is loaded.

That shouldn't cause any confusion. Either a wallet is visible or not.

It's not clear yet what we're going to do about the default wallet, e.g. do we force users to give it a name? I expect only a fraction of users to care about the multi-wallet stuff, so a term like "default wallet" is confusing.

promag · 2019-01-15T16:26:41Z

@Sjors now with your suggestion.

Sjors · 2019-01-15T17:53:09Z

tACK fe7048b

jonasschnelli · 2019-01-15T19:01:23Z

Tested ACK fe7048b

fe7048b gui: Show current wallet name in window title (João Barbosa) 8a79261 gui: Keep network style in BitcoinGUI (João Barbosa) f411c8b gui: Remove unused return type in some BitcoinGUI methods (João Barbosa) Pull request description: <img width="876" alt="screenshot 2019-01-11 at 23 58 26" src="proxy.php?url=https://user-images.githubusercontent.com/3534524/51065458-d7ebaf80-15fc-11e9-9162-e37e9a10d448.png"> Tree-SHA512: 5c43f615834983bc1c5045e07c6e119044dd78ca947fd2679d302b519d5ce1d08d29ca00b1c11e88c4bbc4d56f2e6f4a8adc42084f3503e751e642e8a13112dc

Sjors · 2019-01-19T15:06:12Z

Oops, now we have a trailing - on mainnet.

@Sjors

1ed425e gui: Fix window title update (João Barbosa) Pull request description: Removes trailing `-` from window title when running on mainnet. Reported by @Sjors in #15149 (comment). Tree-SHA512: 22f13c361496720f30a4926d928851ed74456c0d70bd313b0ebaca91a9ebfde96991091ac3d1b094f33d3ce9afafd709eb1917f00d96fa3ca69751b6b14e1d2b

fe7048b gui: Show current wallet name in window title (João Barbosa) 8a79261 gui: Keep network style in BitcoinGUI (João Barbosa) f411c8b gui: Remove unused return type in some BitcoinGUI methods (João Barbosa) Pull request description: <img width="876" alt="screenshot 2019-01-11 at 23 58 26" src="proxy.php?url=https://user-images.githubusercontent.com/3534524/51065458-d7ebaf80-15fc-11e9-9162-e37e9a10d448.png"> Tree-SHA512: 5c43f615834983bc1c5045e07c6e119044dd78ca947fd2679d302b519d5ce1d08d29ca00b1c11e88c4bbc4d56f2e6f4a8adc42084f3503e751e642e8a13112dc # Conflicts: # src/qt/bitcoingui.cpp # src/qt/bitcoingui.h

@Sjors

1ed425e gui: Fix window title update (João Barbosa) Pull request description: Removes trailing `-` from window title when running on mainnet. Reported by @Sjors in bitcoin#15149 (comment). Tree-SHA512: 22f13c361496720f30a4926d928851ed74456c0d70bd313b0ebaca91a9ebfde96991091ac3d1b094f33d3ce9afafd709eb1917f00d96fa3ca69751b6b14e1d2b

fanquake added the GUI label Jan 12, 2019

promag commented Jan 12, 2019

View reviewed changes

src/qt/bitcoingui.cpp Outdated Show resolved Hide resolved

promag commented Jan 12, 2019

View reviewed changes

src/qt/bitcoingui.h Outdated Show resolved Hide resolved

promag force-pushed the 2019-01-updatewindowtitle branch 3 times, most recently from 506bae3 to 530ec6c Compare January 12, 2019 00:49

promag mentioned this pull request Jan 12, 2019

gui: Add WalletController #15101

Merged

hebasto requested changes Jan 13, 2019

View reviewed changes

src/qt/bitcoingui.cpp Outdated Show resolved Hide resolved

src/qt/bitcoingui.h Outdated Show resolved Hide resolved

promag force-pushed the 2019-01-updatewindowtitle branch from 530ec6c to 104251c Compare January 13, 2019 17:58

promag force-pushed the 2019-01-updatewindowtitle branch from c1e8f74 to b0b2d11 Compare January 14, 2019 22:20

gui: Remove unused return type in some BitcoinGUI methods

f411c8b

promag force-pushed the 2019-01-updatewindowtitle branch from b0b2d11 to 3f46d58 Compare January 15, 2019 14:35

promag added 2 commits January 15, 2019 16:23

gui: Keep network style in BitcoinGUI

8a79261

gui: Show current wallet name in window title

fe7048b

promag force-pushed the 2019-01-updatewindowtitle branch from 3f46d58 to fe7048b Compare January 15, 2019 16:25

jonasschnelli merged commit fe7048b into bitcoin:master Jan 15, 2019

promag deleted the 2019-01-updatewindowtitle branch January 15, 2019 21:39

promag mentioned this pull request Jan 19, 2019

gui: Fix window title update #15210

Merged

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

Conversation

promag commented Jan 12, 2019

Uh oh!

Uh oh!

Uh oh!

jonasschnelli commented Jan 12, 2019

Uh oh!

promag commented Jan 12, 2019

Uh oh!

DrahtBot commented Jan 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

promag commented Jan 12, 2019

Uh oh!

benthecarman commented Jan 12, 2019

Uh oh!

hebasto commented Jan 13, 2019

Uh oh!

Uh oh!

Uh oh!

luke-jr commented Jan 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

promag commented Jan 13, 2019

Uh oh!

jonasschnelli commented Jan 13, 2019

Uh oh!

hebasto commented Jan 14, 2019

Uh oh!

promag commented Jan 14, 2019

Uh oh!

hebasto commented Jan 14, 2019

Uh oh!

promag commented Jan 14, 2019

Uh oh!

laanwj commented Jan 14, 2019

Uh oh!

hebasto commented Jan 14, 2019

Uh oh!

Sjors commented Jan 15, 2019

Uh oh!

promag commented Jan 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

promag commented Jan 15, 2019

Uh oh!

Sjors commented Jan 15, 2019

Uh oh!

promag commented Jan 15, 2019

Uh oh!

Sjors commented Jan 15, 2019

Uh oh!

jonasschnelli commented Jan 15, 2019

Uh oh!

Sjors commented Jan 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

DrahtBot commented Jan 12, 2019 •

edited

Loading

luke-jr commented Jan 13, 2019 •

edited

Loading

promag commented Jan 15, 2019 •

edited

Loading