Numerous Qt fixes, qml refactoring, translation escape fixes #1012
+9,554
−5,459
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
UseSavedSettingsPopup.qml and MsgPopup.qml now inherit from it. Reduces code duplication, hardcoded duplicated sizes, etc. MsgPopup now also gets a "pointing hand" cursor for its close button, didn't have before due to oversight, now it inherits from ImPopup.
Makes qmllint happy and allows us to make such warnings fatal soon, to catch real bugs.
Refactor popups
One file per tab. Besides reducing the size of OptionsPopup.qml it's also nice that each tab has an "interface" with public properties while hiding the private impl. For OptionsMiscTab.qml I've improved semantics by only exposing a bool telemetryEnabled instead of the whole check-box, which was very verbose in the caller site. In a followup the 3 tabs will inherit from a base component, so we can share the scrolling behaviour properties
Warnings are fatal, any warning will make a CI run fail. We have 3 categories which are set to "Info" level. These will gradually be bumped to "Warning" level once we fix the warnings. qmllint will be called by CI
So they can share a few properties
Items inside a layout should not set size. The Image size was even misleading, since it's actually rendered as 40x40 (aka its implicit size). This was caught by qmllint: "Detected height on an item that is managed by a layout. This is undefined behavior; use implictHeight or Layout.preferredHeight instead. [Quick.layout-positioning]"
So we don't get into weird situations where it disappears into the background
CI will catch any future unused imports
less qmllint noise
Fix undefined behaviour regarding layouting
Split OptionsPopup.qml into multiple files
This is the modern way to use QML.
- Adds cmake target to run qmlint during build
- C++ type registration is now automatic
- No more manually writing qmldir files and type files
- Allows to use QML_ELEMENT and friends, which allows tooling to
warn us when calling unknown methods (from QML to C++), ie:
proper code-model in QML when using C++ types.
- No need to manually add .qml files to qrc
- Can enable qmlcachegen, disabled for now.
- All kind of options you can toggle, just check docs for
qt_add_qml_module
Tidied the qml.qrc, now separates C++ related assets from QML ones.
Also since the QML assets need to live in the same prefix as the
module.
ImageWriter and DriveListModel were exposed to QML but they were not using QML_ELEMENT annotation. This unleashes the full power of the QML codemodel which now knows about the C++ methods. qmllint can warn about missing methods or typos. In a way it makes our QML usage more "statically typed". Port away from context properties, otherwise none of the above works. And also since context properties are discouraged since Qt 6.
For now just stores the FontLoader. Already helps silencing loads of qmllint warnings regarding unqualified access to the font loader.
Each popup is now self-contained in its own file, bundling the delegate the model and the view and only exposing what main.qml really needs to access.
Removes good chunk of code triplication
Setting QT_QML_GENERATE_QMLLS_INI creates an ini file inside the source directory, which point to the module's location
The warning was: main.qml:425:17 Parameter "drop" is not declared. Injection of parameters into signal handlers is deprecated. Use JavaScript functions with formal parameters instead. The error was about mimedata being undefined, caught by qmllint
CMakePresets create the build directory inside src, so ignore it here. Also ignore .qmlls.ini which is created by CMake now to help with IDE code model
Since we've fixed them all we can mark them as fatal. Whenever someone introduces a problem, CI will yell at us.
Use qt_add_qml_module
Add a Style singleton
Split main.qml into multiple files
Fix all qmllint warnings
Keys.onEscapePressed only works for Item, which Window is not.
Fix OptionsPopup not honouring Esc key
Qt containers detach (copy on write) when a non-const method are called on them. Simply make the container const so the for loop calles cbegin() instead of begin(), which prevents detach.
QRegularExpression is expensive to create, so cache it
Use value() instead of operator[] as it doesn't detach the container
Usually it's dangerous to capture local variables inside a connect, as the connect might call the lambda after those variables went out of scope. In this case it's fine as the connect is disconnected before we leave the function, as we wait for QProcess to end locally
Don't allocate memory (due to temporary containers) when accessing data. values() is usually discouraged, it copies all the data internally into a QList. With an iterator we can access the data in place.
Includes like <QtNetwork> are module-wide includes, which include up to 50 other includes. Include only the specific headers we really need to avoid extra compiler work.
Fix clazy warnings
This doesn't change the design of the old button, but removes the duplicate implementation.
They're now in a central place where designers can conveniently change. Also reduces duplication, since many items now reuse the same style key.
Allows us to use declarative bindings and less imperative JavaScript: - hwlist.currentIndex now has a proper binding to C++ and is not assigned in random GUI places - OSPopup doesn't need to access HWPopup.qml internals anymore, as the hw list is auto sufficient now. - HWListModel now directly passes the filtered tags to ImageWritter, as this is business logic, the gui code doesn't need to be involved.
In JavaScript it's "double"
This allows to remove loads of JavaScript, which is not a type-safe language. Also allows to remove interdependencies/workarounds regarding sharing popup internals with each other. Now the C++ model is the source of truth. While moving from QML to C++, it was important to preserve the model structure, to avoid too much rewrite in a single commit and not create regressions. The model remains flat, as that's the less intrusive way to adapt the GUI. We can now consider a true tree-view model, but needs to be evaluated for GUI rework impact.
Notify main.qml via a signal. No need to pass external buttons and list views into the popups anymore. This makes them more self-contained.
This makes it consistent with the other 2 C++ models. Since it's now invocable, it needs a QObject parent to avoid deletion by garbage collector.
Fixes a qmllint warning about unqualified access. Further increases decoupling. The embedded block could be moved to C++ but I don't have a way to test the embedded mode, so don't want to risk regressions there.
Since we don't have control of what actually is in the JSON list, check for the translation variant as well, to be safe.
Whenever moving strings around you should call this target, so .ts files are updated, so translators can act on them. Also fixes some tr()s not working after they moved from QML to C++
Fixes some tr()s not working after they moved from QML to C++. The command was: ninja rpi-imager_lupdate
Accepting KDAB Changes to rpi-imager. Signed-off-by: Tom Dewey <[email protected]>
This will only work for CMake modules that obey [SYSTEM]_IGNORE_PATH, but it's better than nothing.
As we don't need to modify nghttp2, use the FetchContent mechanism to obtain the code for nghttp2. Note that this isn't a voilation of my vendoring policy - the policy is designed to ensure we know what version is being built - not that it can be built entirely offline at all times.
A missing move command in the entirely-unsigned path would result in a CMake failure. Note that this isn't the most useful target - you can't run an unsigned application on Apple Silicon, but you _can_ build it to confirm at least that stage works.
lurch
reviewed
Feb 26, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.