-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Move IInteractive into separate module #31810
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
base: master
Are you sure you want to change the base?
Conversation
b49bbfb to
361db8f
Compare
361db8f to
33c1a88
Compare
src/framework/ui/uimodule.cpp
Outdated
| @@ -94,7 +93,6 @@ void UiModule::registerExports() | |||
| ioc()->registerExport<IUiConfiguration>(moduleName(), m_configuration); | |||
| ioc()->registerExport<IUiEngine>(moduleName(), m_uiengine); | |||
| ioc()->registerExport<IInteractiveProvider>(moduleName(), m_uiengine->interactiveProvider()); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only InteractiveProvider is too tightly coupled to the ui module to be able to move it.
Could you please remove the interactiveProvider from uiengine... Make interactiveProvider as a regular QML element and create it in InteractiveProvider.qml. This is also necessary for multi-window.
InteractiveProvider.qml
Item {
id: root
property var topParent: null
property alias provider: _provider
...
CppInteractiveProvider {
id: _provider
}
InteractiveProvider::InteractiveProvider()
: QObject(), Injectable(muse::iocCtxForQmlObject(this))
{
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working on this; the challenge is https://github.com/MuseScore/MuseScore/blob/ae198102bc182ab62ddfdf7ab131442f40329582/src/framework/ui/uimodule.cpp#L96.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got it... then I think so
- CppInteractiveProvider - regular service, in internal (not part of the Qml module), but inherited from QObject
- Add an accessor to the Qml module of interactive
InteractiveModule::::registerExports()
{
ioc()->registerExport<IInteractiveProvider>(moduleName(), new InteractiveProvider());
}
class InteractiveProviderAccessor : public QObject
{
QML_ELEMENT
Q_PROPERTY(InteractiveProvider * provider READ provider CONSTANT)
Inject<IInteractiveProvider> interactiveProvider;
}
InteractiveProvider * InteractiveProviderAccessor::provider()
{
return qobject_cast<InteractiveProvider>(interactiveProvider().get());
}
Item {
id: root
property var topParent: null
property alias provider: accessor.provider
...
InteractiveProviderAccessor {
id: accessor
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've gone a step further: instead of exposing the InteractiveProvider object itself to QML, its methods and signals are provided via InteractiveProviderModel.
And then I decided there is no good reason to keep InteractiveProvider separate from Interactive, so I merged them.
Possible future steps:
- Eliminate InteractiveProvider.qml - it doesn't seem to do anything that cannot be done in C++
- Rename InteractiveProvider... but to what?
33c1a88 to
3ae3a6c
Compare
Some parts were in global, some parts in ui, some parts in ui/qml/Muse/Ui/Dialogs. Move as much as possible into a new `interactive` module. Only `InteractiveProvider` is too tightly coupled to the `ui` module to be able to move it.
and make it internal, and introduce InteractiveProviderModel to expose functionality to QML.
Instead of having one interface implementation that defers to another interface for a subset of its functionality, let’s have one implementation of two interfaces: IInteractive as external-facing, IInteractiveProvider as internal.
3ae3a6c to
0364adf
Compare
Some parts were in global, some parts in ui, some parts in ui/qml/Muse/Ui/Dialogs. Move as much as possible into a new
interactivemodule. OnlyInteractiveProvideris too tightly coupled to theuimodule to be able to move it.