Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix setting window title as modified #11542

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

varjolintu
Copy link
Member

@varjolintu varjolintu commented Dec 8, 2024

When creating a new entry to database, the window title does not include an asterisk character. When going to Settings / Password generator, the title appears correctly.

Fixes #11540

Testing strategy

Manually. Switching tab to a closed database, or Settings / Password generator removes the asterisk character from the title.

Type of change

  • ✅ Bug fix (non-breaking change that fixes an issue)

@varjolintu varjolintu added the bug label Dec 8, 2024
@varjolintu varjolintu added this to the v2.7.10 milestone Dec 8, 2024
@varjolintu varjolintu marked this pull request as draft December 8, 2024 11:24
@varjolintu
Copy link
Member Author

varjolintu commented Dec 8, 2024

Noticed that this fix works, but Qt gives a warning to the console: QWidget::setWindowModified: The window title does not contain a '[*]' placeholder. If we want to get rid of that warning too, another solution must be found.

Even if MainWindow::updateWindowTitle() has isModified set to false, in some cases the title will still include the asterisk character.

if (isModified) {
// remove asterisk '*' from title
if (!isModified) {
// Remove asterisk '*' from title
customWindowTitlePart.remove(customWindowTitlePart.size() - 1, 1);
Copy link
Member

Choose a reason for hiding this comment

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

This line is not good because it assumes an asterisk is the very last character.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I have already changed this to .replace() in my test branch. That line also removed the last ] in some cases even without this change.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps an adjustment to the tab widget function is in order as well, I briefly looked at it and seemed too much

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems that we need a new can after the worms are all out.

if (isModified) {
// remove asterisk '*' from title
if (!isModified) {
// Remove asterisk '*' from title
customWindowTitlePart.remove(customWindowTitlePart.size() - 1, 1);
}
m_ui->actionDatabaseSave->setEnabled(m_ui->tabWidget->canSave(tabWidgetIndex));
Copy link
Member

Choose a reason for hiding this comment

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

This should be handled within the action state function instead of here.

@droidmonkey
Copy link
Member

droidmonkey commented Dec 8, 2024

https://doc.qt.io/qt-5/qwidget.html#windowModified-prop

The window title must contain a "[]" placeholder, which indicates where the '' should appear. Normally, it should appear right after the file name (e.g., "document1.txt[*] - Text Editor"). If the window isn't modified, the placeholder is simply removed.

The placeholder should be in the window title no matter what.

@droidmonkey droidmonkey added pr: bugfix Pull request that fixes a bug user interface and removed bug labels Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: bugfix Pull request that fixes a bug user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Window title change indicator (*) not updated in time
2 participants