From c9dea4482bede70e8b0134318e7e7b15e6695b59 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Sep 2025 06:22:20 -0400 Subject: [PATCH] Fix pre-release issues with attachment viewer (#12244) * Fix translation issues for "FIT" and "New Attachment" in attachment editor * Fix markdown preview persistence and enable external links in attachment editor * Update preview panel if manually moved from collapsed position * Match edit view scroll position (by percentage) when changed. This ensures the preview remains in relative sync with the edited document, for example when a large amount of HTML reduces down to a short preview document. * Fix default preview size to be half the width of the edit widget. * Set tab stop to 10 and remove base ui file --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Jonathan White --- share/translations/keepassxc_en.ts | 12 ++-- src/gui/entry/EntryAttachmentsWidget.cpp | 3 +- .../attachments/ImageAttachmentsWidget.cpp | 9 ++- .../attachments/TextAttachmentsEditWidget.cpp | 7 ++- .../attachments/TextAttachmentsEditWidget.h | 1 + .../attachments/TextAttachmentsEditWidget.ui | 6 +- .../TextAttachmentsPreviewWidget.cpp | 37 ++++++++++-- .../TextAttachmentsPreviewWidget.h | 4 ++ .../TextAttachmentsPreviewWidget.ui | 6 +- .../attachments/TextAttachmentsWidget.cpp | 60 ++++++++++++++----- .../entry/attachments/TextAttachmentsWidget.h | 12 ++-- .../attachments/TextAttachmentsWidget.ui | 36 ----------- .../attachments/TestTextAttachmentsWidget.cpp | 10 +++- 13 files changed, 126 insertions(+), 77 deletions(-) delete mode 100644 src/gui/entry/attachments/TextAttachmentsWidget.ui diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts index 643bf9c55..7e8ce9144 100644 --- a/share/translations/keepassxc_en.ts +++ b/share/translations/keepassxc_en.ts @@ -4005,6 +4005,10 @@ Would you like to overwrite the existing attachment? Save… + + New Attachment + + EntryAttributesModel @@ -4574,6 +4578,10 @@ You can enable the DuckDuckGo website icon service in the security section of th Zoom: + + Fit + + ImportWizard @@ -8847,10 +8855,6 @@ This option is deprecated, use --set-key-file instead. TOTP - - Fit - - %1 character(s) diff --git a/src/gui/entry/EntryAttachmentsWidget.cpp b/src/gui/entry/EntryAttachmentsWidget.cpp index 52af98ad6..523850010 100644 --- a/src/gui/entry/EntryAttachmentsWidget.cpp +++ b/src/gui/entry/EntryAttachmentsWidget.cpp @@ -37,7 +37,6 @@ namespace { - constexpr const char* DefaultName = "New Attachment"; constexpr const char* Suffix = ".txt"; QString generateUniqueName(const QString& name, const QStringList& existingNames) @@ -212,7 +211,7 @@ void EntryAttachmentsWidget::newAttachments() } // Create a temporary file to allow the user to edit the attachment - auto newFileName = generateUniqueName(DefaultName, m_entryAttachments->keys()); + auto newFileName = generateUniqueName(tr("New Attachment"), m_entryAttachments->keys()); m_entryAttachments->set(newFileName, QByteArray()); auto currentIndex = m_attachmentsModel->index(m_attachmentsModel->rowByKey(newFileName), 0); diff --git a/src/gui/entry/attachments/ImageAttachmentsWidget.cpp b/src/gui/entry/attachments/ImageAttachmentsWidget.cpp index 3d547433c..4b8f81113 100644 --- a/src/gui/entry/attachments/ImageAttachmentsWidget.cpp +++ b/src/gui/entry/attachments/ImageAttachmentsWidget.cpp @@ -37,8 +37,6 @@ namespace constexpr std::array ZoomList = {0.25, 0.5, 0.75, 1.0, 2.0}; constexpr double WheelZoomStep = 1.1; - const QString FitText = QObject::tr("Fit"); - QString formatZoomText(double zoomFactor) { return QString("%1%").arg(QString::number(zoomFactor * 100, 'f', 0)); @@ -92,9 +90,10 @@ void ImageAttachmentsWidget::initZoomComboBox() { m_ui->zoomComboBox->clear(); - auto textWidth = m_ui->zoomComboBox->fontMetrics().horizontalAdvance(FitText); + auto fitText = tr("Fit"); + auto textWidth = m_ui->zoomComboBox->fontMetrics().horizontalAdvance(fitText); - m_ui->zoomComboBox->addItem(FitText, 0.0); + m_ui->zoomComboBox->addItem(fitText, 0.0); for (const auto& zoom : ZoomList) { auto zoomText = formatZoomText(zoom); @@ -146,7 +145,7 @@ void ImageAttachmentsWidget::onZoomChanged(const QString& zoomText) { auto zoomFactor = 1.0; - if (zoomText == FitText) { + if (zoomText == tr("Fit")) { m_ui->imagesView->enableAutoFitInView(); zoomFactor = std::min(m_ui->imagesView->calculateFitInViewFactor(), zoomFactor); diff --git a/src/gui/entry/attachments/TextAttachmentsEditWidget.cpp b/src/gui/entry/attachments/TextAttachmentsEditWidget.cpp index 1c0bcd223..c67404a6f 100644 --- a/src/gui/entry/attachments/TextAttachmentsEditWidget.cpp +++ b/src/gui/entry/attachments/TextAttachmentsEditWidget.cpp @@ -19,8 +19,8 @@ #include "ui_TextAttachmentsEditWidget.h" #include +#include #include -#include TextAttachmentsEditWidget::TextAttachmentsEditWidget(QWidget* parent) : QWidget(parent) @@ -30,6 +30,11 @@ TextAttachmentsEditWidget::TextAttachmentsEditWidget(QWidget* parent) connect(m_ui->attachmentsTextEdit, &QTextEdit::textChanged, this, &TextAttachmentsEditWidget::textChanged); connect(m_ui->previewPushButton, &QPushButton::clicked, this, &TextAttachmentsEditWidget::previewButtonClicked); + connect(m_ui->attachmentsTextEdit->verticalScrollBar(), &QScrollBar::valueChanged, this, [this](int value) { + // Return a percentage of document scroll + auto percent = static_cast(value) / m_ui->attachmentsTextEdit->verticalScrollBar()->maximum(); + emit scrollChanged(percent); + }); } TextAttachmentsEditWidget::~TextAttachmentsEditWidget() = default; diff --git a/src/gui/entry/attachments/TextAttachmentsEditWidget.h b/src/gui/entry/attachments/TextAttachmentsEditWidget.h index 0879d9251..dfd63fb92 100644 --- a/src/gui/entry/attachments/TextAttachmentsEditWidget.h +++ b/src/gui/entry/attachments/TextAttachmentsEditWidget.h @@ -39,6 +39,7 @@ public: signals: void textChanged(); + void scrollChanged(double percent); void previewButtonClicked(bool isChecked); private: diff --git a/src/gui/entry/attachments/TextAttachmentsEditWidget.ui b/src/gui/entry/attachments/TextAttachmentsEditWidget.ui index e73894d15..fe1fd50ba 100644 --- a/src/gui/entry/attachments/TextAttachmentsEditWidget.ui +++ b/src/gui/entry/attachments/TextAttachmentsEditWidget.ui @@ -57,7 +57,11 @@ - + + + 10.000000000000000 + + diff --git a/src/gui/entry/attachments/TextAttachmentsPreviewWidget.cpp b/src/gui/entry/attachments/TextAttachmentsPreviewWidget.cpp index 4775f289e..99c03ffe7 100644 --- a/src/gui/entry/attachments/TextAttachmentsPreviewWidget.cpp +++ b/src/gui/entry/attachments/TextAttachmentsPreviewWidget.cpp @@ -23,8 +23,10 @@ #include #include #include +#include #include #include +#include namespace { @@ -48,6 +50,7 @@ namespace TextAttachmentsPreviewWidget::TextAttachmentsPreviewWidget(QWidget* parent) : QWidget(parent) , m_ui(new Ui::TextAttachmentsPreviewWidget()) + , m_userManuallySelectedType(false) { m_ui->setupUi(this); @@ -88,10 +91,13 @@ void TextAttachmentsPreviewWidget::initTypeCombobox() filterProxyMode->sort(0, Qt::SortOrder::DescendingOrder); m_ui->typeComboBox->setModel(filterProxyMode); - connect(m_ui->typeComboBox, - QOverload::of(&QComboBox::currentIndexChanged), - this, - &TextAttachmentsPreviewWidget::onTypeChanged); + connect(m_ui->typeComboBox, QOverload::of(&QComboBox::currentIndexChanged), this, [this](int index) { + m_userManuallySelectedType = true; + onTypeChanged(index); + }); + + // Configure text browser to open external links + m_ui->previewTextBrowser->setOpenExternalLinks(true); m_ui->typeComboBox->setCurrentIndex(m_ui->typeComboBox->findData(PlainText)); @@ -100,11 +106,17 @@ void TextAttachmentsPreviewWidget::initTypeCombobox() void TextAttachmentsPreviewWidget::updateUi() { - if (!m_attachment.name.isEmpty()) { + // Only auto-select format based on file extension if user hasn't manually chosen one + if (!m_userManuallySelectedType && !m_attachment.name.isEmpty()) { const auto mimeType = Tools::getMimeType(QFileInfo(m_attachment.name)); auto index = m_ui->typeComboBox->findData(ConvertToPreviewTextType(mimeType)); - m_ui->typeComboBox->setCurrentIndex(index); + if (index >= 0) { + // Temporarily block signals to avoid triggering manual selection flag + m_ui->typeComboBox->blockSignals(true); + m_ui->typeComboBox->setCurrentIndex(index); + m_ui->typeComboBox->blockSignals(false); + } } onTypeChanged(m_ui->typeComboBox->currentIndex()); @@ -116,6 +128,7 @@ void TextAttachmentsPreviewWidget::onTypeChanged(int index) qWarning() << "TextAttachmentsPreviewWidget: Unknown text format"; } + const auto scrollPos = m_ui->previewTextBrowser->verticalScrollBar()->value(); const auto fileType = m_ui->typeComboBox->itemData(index).toInt(); if (fileType == TextAttachmentsPreviewWidget::PreviewTextType::PlainText) { m_ui->previewTextBrowser->setPlainText(m_attachment.data); @@ -130,4 +143,16 @@ void TextAttachmentsPreviewWidget::onTypeChanged(int index) m_ui->previewTextBrowser->setMarkdown(m_attachment.data); } #endif + + // Delay setting the scrollbar position to ensure the text is rendered first + QTimer::singleShot( + 100, this, [this, scrollPos] { m_ui->previewTextBrowser->verticalScrollBar()->setValue(scrollPos); }); +} + +void TextAttachmentsPreviewWidget::matchScroll(double percent) +{ + // Match the scroll position of the text browser to the given percentage + int maxScroll = m_ui->previewTextBrowser->verticalScrollBar()->maximum(); + int newScrollPos = static_cast(maxScroll * percent); + m_ui->previewTextBrowser->verticalScrollBar()->setValue(newScrollPos); } diff --git a/src/gui/entry/attachments/TextAttachmentsPreviewWidget.h b/src/gui/entry/attachments/TextAttachmentsPreviewWidget.h index c7d20eb9d..737a56732 100644 --- a/src/gui/entry/attachments/TextAttachmentsPreviewWidget.h +++ b/src/gui/entry/attachments/TextAttachmentsPreviewWidget.h @@ -48,6 +48,9 @@ public: Q_ENUM(PreviewTextType) +public slots: + void matchScroll(double percent); + private slots: void onTypeChanged(int index); @@ -58,4 +61,5 @@ private: QScopedPointer m_ui; attachments::Attachment m_attachment; + bool m_userManuallySelectedType; }; diff --git a/src/gui/entry/attachments/TextAttachmentsPreviewWidget.ui b/src/gui/entry/attachments/TextAttachmentsPreviewWidget.ui index 2ee97aa22..2f2ea14a1 100644 --- a/src/gui/entry/attachments/TextAttachmentsPreviewWidget.ui +++ b/src/gui/entry/attachments/TextAttachmentsPreviewWidget.ui @@ -61,7 +61,11 @@ - + + + 10.000000000000000 + + diff --git a/src/gui/entry/attachments/TextAttachmentsWidget.cpp b/src/gui/entry/attachments/TextAttachmentsWidget.cpp index d0935c6af..b28465a46 100644 --- a/src/gui/entry/attachments/TextAttachmentsWidget.cpp +++ b/src/gui/entry/attachments/TextAttachmentsWidget.cpp @@ -19,19 +19,16 @@ #include "TextAttachmentsEditWidget.h" #include "TextAttachmentsPreviewWidget.h" -#include "ui_TextAttachmentsWidget.h" - #include #include #include +#include TextAttachmentsWidget::TextAttachmentsWidget(QWidget* parent) : QWidget(parent) - , m_ui(new Ui::TextAttachmentsWidget()) , m_previewUpdateTimer(new QTimer(this)) , m_mode(attachments::OpenMode::ReadOnly) { - m_ui->setupUi(this); initWidget(); } @@ -57,15 +54,25 @@ attachments::Attachment TextAttachmentsWidget::getAttachment() const void TextAttachmentsWidget::updateWidget() { if (m_mode == attachments::OpenMode::ReadOnly) { + // Only show the preview widget in read-only mode m_splitter->setSizes({0, 1}); m_editWidget->hide(); + m_previewWidget->openAttachment(m_attachment, m_mode); } else { + // Show the edit widget and hide the preview by default in read-write mode m_splitter->setSizes({1, 0}); m_editWidget->show(); + m_editWidget->openAttachment(m_attachment, m_mode); } +} - m_editWidget->openAttachment(m_attachment, m_mode); - m_previewWidget->openAttachment(m_attachment, attachments::OpenMode::ReadOnly); +void TextAttachmentsWidget::updatePreviewWidget() +{ + m_previewVisible = isPreviewVisible(); + if (m_previewVisible) { + m_attachment = m_editWidget->getAttachment(); + m_previewWidget->openAttachment(m_attachment, attachments::OpenMode::ReadOnly); + } } void TextAttachmentsWidget::initWidget() @@ -78,20 +85,31 @@ void TextAttachmentsWidget::initWidget() m_previewUpdateTimer->setInterval(500); // Only update the preview after a set timeout and if it is visible - connect(m_previewUpdateTimer, &QTimer::timeout, this, [this] { - if (m_previewWidget->width() > 0) { - m_attachment = m_editWidget->getAttachment(); - m_previewWidget->openAttachment(m_attachment, attachments::OpenMode::ReadOnly); - } - }); + connect(m_previewUpdateTimer, &QTimer::timeout, this, &TextAttachmentsWidget::updatePreviewWidget); + connect(m_editWidget, + &TextAttachmentsEditWidget::scrollChanged, + m_previewWidget, + &TextAttachmentsPreviewWidget::matchScroll); connect( m_editWidget, &TextAttachmentsEditWidget::textChanged, m_previewUpdateTimer, QOverload<>::of(&QTimer::start)); connect(m_editWidget, &TextAttachmentsEditWidget::previewButtonClicked, [this] { - const auto sizes = m_splitter->sizes(); - const auto previewSize = sizes.value(1, 0) > 0 ? 0 : 1; - m_splitter->setSizes({1, previewSize}); + // Split the display in half if showing the preview widget + const auto previewSize = isPreviewVisible() ? 0 : m_splitter->width() / 2; + const auto editSize = m_splitter->width() - previewSize; + m_splitter->setSizes({editSize, previewSize}); + updatePreviewWidget(); + }); + + // Check if the preview panel is manually collapsed or shown + connect(m_splitter, &QSplitter::splitterMoved, this, [this](int, int) { + // Trigger a preview update if it has become visible + auto visible = isPreviewVisible(); + if (visible && !m_previewVisible) { + updatePreviewWidget(); + } + m_previewVisible = visible; }); m_splitter->addWidget(m_editWidget); @@ -99,7 +117,17 @@ void TextAttachmentsWidget::initWidget() // Prevent collapsing of the edit widget m_splitter->setCollapsible(0, false); - m_ui->verticalLayout->addWidget(m_splitter); + // Setup this widget with the splitter + auto layout = new QVBoxLayout(this); + layout->setContentsMargins(0, 0, 0, 0); + layout->addWidget(m_splitter); + setLayout(layout); + setObjectName("TextAttachmentsWidget"); updateWidget(); } + +bool TextAttachmentsWidget::isPreviewVisible() const +{ + return m_previewWidget->width() > 0; +} diff --git a/src/gui/entry/attachments/TextAttachmentsWidget.h b/src/gui/entry/attachments/TextAttachmentsWidget.h index f4709823f..048ab7c50 100644 --- a/src/gui/entry/attachments/TextAttachmentsWidget.h +++ b/src/gui/entry/attachments/TextAttachmentsWidget.h @@ -43,15 +43,19 @@ public: void openAttachment(attachments::Attachment attachment, attachments::OpenMode mode); attachments::Attachment getAttachment() const; -private: - void updateWidget(); - void initWidget(); +private slots: + void updatePreviewWidget(); + +private: + void initWidget(); + void updateWidget(); + bool isPreviewVisible() const; - QScopedPointer m_ui; QPointer m_splitter; QPointer m_editWidget; QPointer m_previewWidget; QPointer m_previewUpdateTimer; + bool m_previewVisible = false; attachments::Attachment m_attachment; attachments::OpenMode m_mode; diff --git a/src/gui/entry/attachments/TextAttachmentsWidget.ui b/src/gui/entry/attachments/TextAttachmentsWidget.ui deleted file mode 100644 index 9c18a7c60..000000000 --- a/src/gui/entry/attachments/TextAttachmentsWidget.ui +++ /dev/null @@ -1,36 +0,0 @@ - - - TextAttachmentsWidget - - - - 0 - 0 - 732 - 432 - - - - - - - - 0 - - - 0 - - - 0 - - - 0 - - - 0 - - - - - - diff --git a/tests/gui/attachments/TestTextAttachmentsWidget.cpp b/tests/gui/attachments/TestTextAttachmentsWidget.cpp index bba6f9840..b5bbab324 100644 --- a/tests/gui/attachments/TestTextAttachmentsWidget.cpp +++ b/tests/gui/attachments/TestTextAttachmentsWidget.cpp @@ -50,10 +50,18 @@ void TestTextAttachmentsWidget::testTextReadWriteWidget() QCOMPARE(attachments.name, Test.name); QCOMPARE(attachments.data, Test.data); + // Find preview widget and verify that it is in collapsed state auto previewWidget = qobject_cast(splitter->widget(1)); QVERIFY(previewWidget); - attachments = previewWidget->getAttachment(); + QCOMPARE(previewWidget->width(), 0); + // Show the preview widget + QTest::mouseClick(m_textWidget->findChild("previewPushButton"), Qt::LeftButton); + QCoreApplication::processEvents(); + QVERIFY(previewWidget->width() > 0); + + // Verify attachment data has been loaded in the preview + attachments = previewWidget->getAttachment(); QCOMPARE(attachments.name, Test.name); QCOMPARE(attachments.data, Test.data); }