feat(QML): add basic DnD Deck interface#14849
Conversation
afe82d9 to
bd2e88d
Compare
afe82d9 to
dd94653
Compare
|
This PR is a little bit large as it adds all the basic deck component, but doesn't bring any new C++ code so hopefully won't be too hard to review! |
dd94653 to
0f2958a
Compare
0f2958a to
dd94653
Compare
27311ce to
789322e
Compare
95d6af5 to
09b5b4d
Compare
09b5b4d to
2e07997
Compare
|
I just tested this an noticed that I can move decks/deck elements when Edit is not active. |
|
I do not see the blue higlighting of the selected elements as in the video above. |
Likely related to the Qt version difference... Could you share a screencast for reference?
You need to press |
|
I'm adding copilot as a reviewer -- I've had decent success with it but by no means will it be necessary to do everything it says. |
There was a problem hiding this comment.
Pull request overview
This pull request adds a basic drag-and-drop deck interface for the QML skin, representing a significant UI/UX enhancement. The changes include a complete refactoring of the deck layout system to support customizable, draggable components with an edit mode.
Key changes:
- New draggable deck component system with edit mode support
- Refactored Mixer to support 4-deck configuration with integrated crossfader
- Introduction of new deck sub-components (Spinny, Loop, BeatJump, HotcueAndStem, etc.)
Reviewed changes
Copilot reviewed 35 out of 37 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| res/qml/main.qml | Major layout restructure using SplitView; added Edit and Aux buttons; restructured waveform displays and deck arrangement |
| res/qml/Deck.qml | Complete refactor to support drag-and-drop interface with serializable layout models and edit mode |
| res/qml/Mixer.qml | Rebuilt for 4-deck support with integrated crossfader and deck assignment buttons |
| res/qml/Deck/*.qml | New modular deck components (InfoBar, Toolbar, TempoColumn, Spinny, PlayButton, Loop, HotcueAndStem, FXAssign, CueButton, BeatJump, WaveformOverview) |
| res/qml/ControlFader.qml | New fader control component replacing ControlSlider in several places |
| res/qml/Mixxx/Controls/Fader.qml | New base fader control with bar visualization |
| res/qml/RangeButton.qml | New button for cycling through tempo range values |
| res/qml/EqKnob.qml | Updated to use double-tap for mute toggle; removed separate status button |
| res/qml/QuickFxKnob.qml | Simplified interface; removed effect selector dropdown |
| res/qml/EqColumn.qml | Removed stem support UI (stems moved to HotcueAndStem) |
| res/qml/Theme/Theme.qml | Added multiple new theme colors for deck components |
| res/qml/Library/*.qml | Minor color adjustments to match new theme |
| res/qml/DeckInfoBar.qml | Deleted - replaced by Deck/InfoBar.qml |
| res/qml/InfoBarButton.qml | Deleted - no longer needed |
| res/qml/DeckRow.qml | Deleted - deck layout now handled in main.qml |
| CMakeLists.txt | Updated to reference new Fader.qml instead of Slider.qml |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
This is very cool and I love seeing how QML is coming along. Most of my comments are questions and clarifications.
I think my primary concern as we forge ahead is that the QML code mixes together logic and style in a way that will make it very hard to debug and/or review and/or support multiple themes or color schemes. My second concern is constant values that are shared between engine code and QML code. Let's figure out a way to keep those in sync, whether it's through some sort of QML->C++ callout or shared header files or something.
I will approve merging this in, but we need to address these questions and I understand it may result in a lot of refactoring and work. But that's something that's a lot easier to do now, and we want to get it right so that QML provides a solid technical foundation for the next 20 years of Mixxx!
|
Note that the current failing CI is unrelated and also happening on |
There was a problem hiding this comment.
obviously I can't review every line but the form is looking good, thank you for finding the formatter and doing the cleanups. Can we find someone to do a quick functionality pass (maybe one of our testers?), and if it's working all right, we can merge.
| PathLine { | ||
| x: 26 | ||
| y: 4 | ||
| } | ||
| PathLine { | ||
| x: 19.5 | ||
| y: 10 | ||
| } | ||
| PathLine { | ||
| x: 19.5 | ||
| y: 24 | ||
| } | ||
| PathLine { | ||
| x: 16.5 | ||
| y: 24 | ||
| } | ||
| PathLine { | ||
| x: 16.5 | ||
| y: 10 | ||
| } | ||
| PathLine { | ||
| x: 10 | ||
| y: 4 | ||
| } |
There was a problem hiding this comment.
nonblocking: it would br nice if the formatter let us keep this compact
There was a problem hiding this comment.
Yeah I agree... I'll see if there is anything we can do about this!
There was a problem hiding this comment.
Unfortunately, it does not look like this is something we can change at the moment, but the qmlformat tool is involving fast, so we may hopefully see this feature in the future.
|
@ywwg Are you okay with me squashing all the fixup or would you want the auto formatting to remain in a dedicated commit? |
|
Can you squash most of it and leave the formatting separate? If not, more commits is better than fewer |
|
Will do - do you want me to setup a .git-blame-ignore-revs or should I leave this as is. Arguably, this is still the early daus of QML so the blame maybe doesn’t matter so much |
f16aa96 to
c020377
Compare
|
@ywwg I have squashed every fixup and kept the formatting as its own commit - hopefully this is enough to get this merged. |
Here is the requested screencast, that shows that moving of a deck is possible without enabling the Edit mode: DeckMoveInNonEditMode.mp4 |
|
Thanks for reporting this issue! I had the same on Android which was quite a pain and thought this was something specific to Android and touchscreen, glad to find it was reproducible on Windows - including in my VM. I have now pushed a fix (+some small tweak as I spotted some little nits). Could you confirm everything looks fine now? |
24d07d1 to
6178d05
Compare
|
All commit squashed - ready for merge! 🚀 |
Partially addresses #14523 and #14524. Serialisation of edited decks to come in #14539
Screencast.From.2025-05-26.03-52-57.mp4
Notes: