Skip to content

Refactor: ContainerWidget optional dragId property to eliminate AppletPanel fallback (follow-up to #1836) #3057

Description

@ten9876

Background

PR #3044 (closes #1836) addressed the TXDSP drag silent-fail by adding an aliasing fallback in `AppletPanel::dropEvent`. The fallback catches the case where a composite tile's internal `ContainerWidget.m_id` (e.g. `tx_dsp`) differs from its owning `AppletEntry.id` (e.g. `TXDSP`).

The fallback is correct and regression-safe — but it's a workaround. The underlying duality of "container internal id" vs "applet panel id" remains. Long-term cleanup is to make the drag identity explicit.

Proposed refactor

Give `ContainerWidget` an optional `m_dragId` property that defaults to `m_id` but can be set explicitly at construction:

```cpp
// src/gui/containers/ContainerWidget.h
class ContainerWidget : public QWidget {
public:
explicit ContainerWidget(const QString& id,
const QString& title,
QWidget* parent = nullptr);
QString id() const { return m_id; }
QString dragId() const { return m_dragId.isEmpty() ? m_id : m_dragId; }
void setDragId(const QString& dragId) { m_dragId = dragId; }
private:
QString m_id;
QString m_dragId; // empty → use m_id; non-empty overrides for drag MIME
};
```

Then in `ContainerWidget::onTitleBarDragStart`:
```cpp
mime->setData("application/x-aethersdr-applet", dragId().toUtf8());
```

And in `AppletPanel` where composite tiles are wrapped:
```cpp
auto entry = makeEntry("TXDSP", "VUDU", txDsp, ...);
if (auto* c = qobject_cast<ContainerWidget*>(txDsp))
c->setDragId("TXDSP");
```

After this lands, the aliasing fallback in `AppletPanel::dropEvent` becomes dead code and can be deleted in the same PR.

Stats

  • ~30 LOC change in `ContainerWidget.{cpp,h}`
  • 1 `setDragId` call in `AppletPanel.cpp` (only the TXDSP wrap site needs it today)
  • Delete the ~15 LOC fallback in `AppletPanel::dropEvent`
  • Net: smaller diff than the aliasing fallback was

Risk

Low — pure refactor, behavior-preserving for non-composite tiles (their `dragId()` returns `m_id` unchanged). TXDSP becomes "drag MIME is TXDSP" matching its `AppletEntry.id` directly — fast path now hits, no fallback needed.

Container persistence keys (`ContainerGeometry_tx_dsp`, `ContainerTree` parent refs) are unaffected because they read `m_id`, not `m_dragId`. The whole point of the `m_dragId` separation is that drag identity is a different concept than container identity.

Eligibility

`aetherclaude-eligible` candidate — well-bounded refactor with clear behavior preservation. Could be one of the bot's next pick-ups.

Spotted by

Bot review on PR #3044 — observation #2: "A follow-up could give ContainerWidget an optional 'drag id' property that defaults to m_id but can be set to the owning AppletEntry.id at construction. That would let the fast path always succeed and let the fallback be deleted."

Metadata

Metadata

Assignees

No one assigned

    Labels

    GUIUser interfaceenhancementImprovement to existing featuremaintainer-reviewRequires maintainer review before any action is takenrefactorCode cleanup, restructuring, or consolidation — no user-visible behavior change

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions