Zandronum Chat @ irc.zandronum.com
#zandronum
Get the latest version: 3.0
Source Code

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0003482Doomseeker[All Projects] Suggestionpublic2018-09-01 11:222018-09-20 16:41
ReporterWubTheCaptain 
Assigned ToPol M 
PrioritynormalSeveritytweakReproducibilityalways
Statusneeds testingResolutionopen 
PlatformOSOS Version
Product Version1.1 
Target Version1.2Fixed in Version 
Summary0003482: Add visual text indication to Appearance configuration if/when the program needs to be restarted for changes to apply (locales)
DescriptionSince commit acb43c63c19086f8d592380b66cbb43d77f76366 in Doomseeker 1.2~beta, there's a line printed to log (stdout) telling the program needs to be restarted. In regular use, this line is unfortunately not often to be seen. The user could be left slightly clueless, deprived of any hint that the program needs to be restarted for changes to apply in entirety.

I figured there's also no visual text indication when a program needs to be restarted in Doomseeker 1.1 for language changes to take effect, if this issue even applies there.

Display a warning message in "Appearance" configuration when the appropriate conditions exist for program restart required.
Steps To Reproduce
  1. Start Doomseeker.
  2. Press the F5 key to open the configuration or find Options → Configuration from the top crate navigation.
  3. Navigate to "Appearance" settings.
  4. Select another language from the dropdown menu.
  5. Press "Ok" or "Apply".
  6. Notice many parts of the GUI are not translated, such as server filter.
  7. Optionally, revert back to the language prior (e.g., English) and apply changes. Some parts will still be or won't be translated.
  8. A program restart is required, but there is no warning message in the "Appearance" configuration window to say this in a foolproof way.
Additional Informationsrc/core/gui/configuration/cfgappearance.cpp CFGAppearance::saveSettings().
Attached Files

- Relationships
related to 0003260feedbackZalewa Translation "en_US" cannot be found, unknown language definition 
related to 0003483acknowledged Doomseeker 1.2 release 

-  Notes
User avatar (0019520)
Pol M (developer)
2018-09-13 20:11

pull request
User avatar (0019524)
Zalewa (developer)
2018-09-15 07:41
edited on: 2018-09-15 07:42

I think this works as intended now, with the message box appearing after the config box is closed.

There's a slight misdemeanor when you switch to a different language, apply and then go back to the original one and apply again. Even though you haven't changed the language effectively, the popup still appears, as the partial translation already happens immediately after you press the apply button. It could be delayed to occur after you close the config box and only if the user has effectively changed the locale. Alternative idea is to completely drop the dynamic retranslation code, but we cannot do that as the message box that tells the user to restart the program should appear in the language the user has chosen.

User avatar (0019540)
WubTheCaptain (developer)
2018-09-18 12:14
edited on: 2018-09-18 12:36

Quote
Doomseeker needs to restart to be able to apply some changes


→ "Doomseeker needs to be restarted for some changes to be applied. [Would you like to restart Doomseeker now? [Yes/No]]"

Notice missing period and the copyedit.

The choice is optional, but would have increased usability. Alternatively, see below.

Quote
QMessageBox::warning(NULL, tr("Restart needed"), warningRestartNeeded);


Use of QMessageBox is somewhat inconsistent with other warnings in the Configuration menu, and griefing like Zalewa mentioned. I'd take a look at how Wadseeker's General configuration with missing/invalid paths creates that QPixmap(":/icons/exclamation_16.png") string thingy and add it to the Appearance menu instead when the language option is changed (before pressing "OK" or "Apply"), replacing the popup box.

I think this is more of an enhancement than required functionality for this ticket, though. I would create a new ticket for this.

User avatar (0019543)
Pol M (developer)
2018-09-18 16:31

hey, I've been reading the code and I have noticed the existence of importantMessagesWidget which helps with the creation of small messages, just like this one... I'm quite sure that Zalewa was referring to this, my bad, I had to go to sleep :/
I'm gonna change the QMessage to importantMessagesWidget and change the text to:
Quote from WubTheCaptain

"Doomseeker needs to be restarted for some changes to be applied."


In case that the yes/no buttons are really mandatory, I'd have to add a custom entry in mainwindow.ui, like the server filter text. I personally find it unnecessary.

Also, I tried to extend this to make the message callable in case any future configuration required to restart, instead of having to make other messages pop up in all of those options, but since right now the only plausible one is the locale, I would understand if you prefer the second recommendation.
User avatar (0019544)
Pol M (developer)
2018-09-18 16:55

pull request
User avatar (0019550)
WubTheCaptain (developer)
2018-09-18 22:13
edited on: 2018-09-18 22:24

Pol M: As a sidenote, I think it'd be easier to test your things if you used a single repository with multiple named Mercurial branches. Seehttps://www.mercurial-scm.org/wiki/NamedBranches [^] for more info. Right now you're creating and uploading multiple copies of the Doomseeker repository to Bitbucket with unusual project identifiers, wasting some bandwidth in the process – and causing me a little bit of griefance, because I can't notice/fetch/pull/update your changes from a single repository URL for new branches. In other words: You have 7 different "forks" of Doomseeker, not one; all with a single "default" branch, not seperated branches from upstream.

In other words: I would prefer to pull all your changes for testing fromhttps://bitbucket.org/Pol_M/doomseeker [^] with "default" branch being the upstream, and "blah-whatever-feature-or-fix-you-name" branch being your change for that thing – and you can continue pushing to "blah-whatever-feature-or-fix-you-name" for further changes of the same ticket.

User avatar (0019551)
Pol M (developer)
2018-09-18 22:23

Sorry about that, it just is the quickest way to get my things done. I'm quite new to version control systems where there is more than just me making changes. I'll try to learn about branches for the next ticket, and use them instead of having multiple forks.

Thanks for the link :D
User avatar (0019577)
WubTheCaptain (developer)
2018-09-19 23:56
edited on: 2018-09-19 23:56

Quote from WubTheCaptain
Pol M: As a sidenote, I think it'd be easier to test your things if you used a single repository with multiple named Mercurial branches.


s/branches/bookmarks/, as pointed out by Blutz3. (https://www.mercurial-scm.org/wiki/Bookmarks)

User avatar (0019578)
Pol M (developer)
2018-09-20 05:43

No worries XD
User avatar (0019581)
Zalewa (developer)
2018-09-20 16:15

Let's not add the "Restart? Yes/No" buttons for now, the notification is sufficient. PR has been merged.
User avatar (0019583)
Pol M (developer)
2018-09-20 16:41

I agree with Zalewa tbh :P

Issue Community Support
Only registered users can voice their support. Click here to register, or here to log in.
Supporters: No one explicitly supports this issue yet.
Opponents: No one explicitly opposes this issue yet.

- Issue History
Date Modified Username Field Change
2018-09-01 11:22 WubTheCaptain New Issue
2018-09-01 11:23 WubTheCaptain Relationship added related to 0003260
2018-09-10 05:29 Zalewa Relationship added related to 0003483
2018-09-12 15:03 Pol M Assigned To => Pol M
2018-09-12 15:03 Pol M Status new => assigned
2018-09-13 20:11 Pol M Note Added: 0019520
2018-09-13 20:12 Pol M Status assigned => needs review
2018-09-15 07:41 Zalewa Note Added: 0019524
2018-09-15 07:41 Zalewa Status needs review => needs testing
2018-09-15 07:42 Zalewa Note Edited: 0019524 View Revisions
2018-09-18 11:48 WubTheCaptain Target Version => 1.2
2018-09-18 12:14 WubTheCaptain Note Added: 0019540
2018-09-18 12:14 WubTheCaptain Status needs testing => needs review
2018-09-18 12:17 WubTheCaptain Status needs review => assigned
2018-09-18 12:36 WubTheCaptain Note Edited: 0019540 View Revisions
2018-09-18 12:36 WubTheCaptain Note Edited: 0019540 View Revisions
2018-09-18 16:31 Pol M Note Added: 0019543
2018-09-18 16:55 Pol M Status assigned => needs review
2018-09-18 16:55 Pol M Note Added: 0019544
2018-09-18 22:13 WubTheCaptain Note Added: 0019550
2018-09-18 22:14 WubTheCaptain Note Edited: 0019550 View Revisions
2018-09-18 22:15 WubTheCaptain Note Edited: 0019550 View Revisions
2018-09-18 22:20 WubTheCaptain Note Edited: 0019550 View Revisions
2018-09-18 22:20 WubTheCaptain Note Edited: 0019550 View Revisions
2018-09-18 22:20 WubTheCaptain Note Edited: 0019550 View Revisions
2018-09-18 22:20 WubTheCaptain Note Edited: 0019550 View Revisions
2018-09-18 22:21 WubTheCaptain Note Edited: 0019550 View Revisions
2018-09-18 22:22 WubTheCaptain Note Edited: 0019550 View Revisions
2018-09-18 22:23 Pol M Note Added: 0019551
2018-09-18 22:24 WubTheCaptain Note Edited: 0019550 View Revisions
2018-09-19 23:56 WubTheCaptain Note Added: 0019577
2018-09-19 23:56 WubTheCaptain Note Edited: 0019577 View Revisions
2018-09-20 05:43 Pol M Note Added: 0019578
2018-09-20 16:15 Zalewa Note Added: 0019581
2018-09-20 16:41 Pol M Note Added: 0019583
2018-09-20 16:41 Pol M Status needs review => needs testing






Questions or other issues? Contact Us.

Links


Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker