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

QML: Add developer tools window #4568

Merged
merged 10 commits into from
Dec 15, 2021
Merged

QML: Add developer tools window #4568

merged 10 commits into from
Dec 15, 2021

Conversation

Holzhaus
Copy link
Member

No description provided.

@Holzhaus Holzhaus added the ui label Dec 14, 2021
@Holzhaus Holzhaus force-pushed the qml-devtools branch 2 times, most recently from 65b0022 to c45eadb Compare December 14, 2021 18:07
@github-actions github-actions bot added the build label Dec 14, 2021
@ywwg
Copy link
Member

ywwg commented Dec 14, 2021

lol I should have read this before my comment in #4567 !

@ywwg
Copy link
Member

ywwg commented Dec 15, 2021

I am building qt6 so I can review these -- code looks fine but I'd like to be able to see it for myself

@Holzhaus
Copy link
Member Author

Holzhaus commented Dec 15, 2021

I am building qt6 so I can review these -- code looks fine but I'd like to be able to see it for myself

Oh, this PR is not finished yet. You can see the controls but changing them does not work atm.

Also, Mixxx does not build with Qt6 yet. To test the QML UI, Qt5 suffices. I think current main requires at least Qt 5.12, but this PR needs Qt 5.15.

@Holzhaus
Copy link
Member Author

Holzhaus commented Dec 15, 2021

You can see the controls but changing them does not work atm.

Editing works now, but the values donz update yet. The QWidgets UI polls all controls using a timerEvent, but this does not work with QML and updating them via connections should be more performant anyway.

@Holzhaus Holzhaus marked this pull request as ready for review December 15, 2021 14:10
@Holzhaus
Copy link
Member Author

okay, it's not pretty but it works now. Just build Mixxx as you normally would (using Qt 5.15), then run mixxx using ./mixxx --qml.

Copy link
Member

@ywwg ywwg left a comment

Choose a reason for hiding this comment

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

yeah I figured out how to make it work finally. looks good!

@Holzhaus
Copy link
Member Author

Alright, then feel free to merge.

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

The PR is broken. Seems like you forgot to include TextField.qml and didn't register some types:

warning [Main] QQmlApplicationEngine failed to load component
warning [Main] file:///home/swiftb0y/Sourcerepositories/mixxx/res/qml/main.qml:92:21: Type DeveloperToolsWindow unavailable
warning [Main] file:///home/swiftb0y/Sourcerepositories/mixxx/res/qml/DeveloperToolsWindow.qml:20:9: Type Skin.TextField unavailable
warning [Main] file:///home/swiftb0y/Sourcerepositories/mixxx/res/qml/TextField.qml: No such file or directory
critical [Main] Failed to load QML file "res/qml/main.qml"

@Swiftb0y
Copy link
Member

thanks

@Swiftb0y
Copy link
Member

headings get squeezed when the table is empty when there are no search results. Don't feed to fix it now if its too much of a hassle but then leave a comment please.
image

@Swiftb0y Swiftb0y dismissed their stale review December 15, 2021 18:58

I am currently uncapable to review this properly.

@Holzhaus
Copy link
Member Author

headings get squeezed when the table is empty when there are no search results. Don't feed to fix it now if its too much of a hassle but then leave a comment please.
image

Done.

@ywwg
Copy link
Member

ywwg commented Dec 15, 2021

since this is high-flux developer code I don't think it has to be perfect UI. we can iterate as needed but it sounds like this is needed to unblock QML work

@ywwg ywwg merged commit 9d345cc into mixxxdj:main Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants