Michał
Michał
parent
6cc11bbca9
commit
9acf723ddc
|
|
@ -6,14 +6,10 @@
|
|||
|
||||
<!-- List the affected areas (e.g wallet, browser, etc..) -->
|
||||
|
||||
### StatusQ checklist
|
||||
### Architecture compliance
|
||||
|
||||
- [ ] add documentation if necessary (new component, new feature)
|
||||
- [ ] update sandbox app
|
||||
- in case of new component, add new component page
|
||||
- in case of new features, add variation to existing component page
|
||||
- nice to have: add it to the demo application as well
|
||||
- [ ] test changes in both light and dark theme?
|
||||
- [ ] I am familiar with the application architecture and agreed good practices.
|
||||
My PR is consistent with this document: [Architecture guidelines](../architecture-guide.md)
|
||||
|
||||
### Screenshot of functionality (including design for comparison)
|
||||
|
||||
|
|
@ -41,7 +37,3 @@ Tick **one**:
|
|||
- [ ] High risk: QA team MUST perform additional testing in the specified affected areas **before** merging.
|
||||
|
||||
-->
|
||||
|
||||
### Cool Spaceship Picture
|
||||
|
||||
<!-- optional but cool -->
|
||||
|
|
|
|||
|
|
@ -0,0 +1,435 @@
|
|||
|
||||
# Status Desktop Architecture Guide
|
||||
|
||||
## Introduction
|
||||
|
||||
This document contains a series of architectural decisions, principles, and
|
||||
best practices. They are largely derived from
|
||||
[SOLID](https://en.wikipedia.org/wiki/SOLID), adapted to the needs of
|
||||
the project. Many of them are not applied in the current code, but there is
|
||||
a great need to change this and establish a clear application architecture and
|
||||
improve the quality of the code. The goal is to follow these guideline in new
|
||||
code and gradually fix existing codebase.
|
||||
|
||||
The diagram depicting top-level architecture can be found
|
||||
[here](https://miro.com/app/board/uXjVKS3chcc=/).
|
||||
|
||||
## General architecture good practices
|
||||
|
||||
- Singletons should be stateless.
|
||||
|
||||
Any state in a singleton is a flaw from the application architecture point
|
||||
of view. This creates a number of problems. First, it causes unit tests to be
|
||||
interdependent, introducing hard to detect bugs. Another, even more important
|
||||
problem is the order of initialization. In code like this:
|
||||
|
||||
```qml
|
||||
Component.onCompleted: {
|
||||
Global.applicationWindow = this // BAD
|
||||
}
|
||||
```
|
||||
|
||||
there is no guarantee that other components will not call `Global.applicationWindow`
|
||||
before this variable is initialized. The order of initialization of QML
|
||||
components and therefore calls to `Component.onCompleted` is undefined.
|
||||
|
||||
- Singletons should not refer to the backend.
|
||||
The only layer that has access to the backend is the layer of stores.
|
||||
Components relying on singletons accessing the backend are hard to test.
|
||||
Backend references used in the singleton must be mocked, which is problematic
|
||||
and requires additional exposure of context properties from the singleton only
|
||||
for testing and storybook purposes, as in the example below.
|
||||
|
||||
```qml
|
||||
// Utils.qml (singleton)
|
||||
|
||||
QtObject {
|
||||
property var mainModuleInst: typeof mainModule !== "undefined" ? mainModule : null
|
||||
property var sharedUrlsModuleInst: typeof sharedUrlsModule !== "undefined" ? sharedUrlsModule : null
|
||||
property var globalUtilsInst: typeof globalUtils !== "undefined" ? globalUtils : null
|
||||
}
|
||||
```
|
||||
|
||||
The testing/storybook code also becomes complicated because it is important to
|
||||
ensure that a singleton is mocked before the tested component is instantiated.
|
||||
|
||||
Stateless singleton also means not using components like `Settings` inside singletons.
|
||||
|
||||
- The API of components should be well thought out. Expose dependencies,
|
||||
hide internal details.
|
||||
|
||||
Ideally, it should be enough for a developer to read the public API of a given
|
||||
component to understand what is needed to use it correctly.
|
||||
|
||||
- Examples of hidden dependencies:
|
||||
- using stateful singletons - components communicate to each other
|
||||
implicitly, dependency is not visible in their public API
|
||||
- communicating by calling signals on global objects
|
||||
- accessing backend via singletons
|
||||
- taking via public API much more than needed (e.g. component needs two
|
||||
properties from store, but the whole store is provided with tens of
|
||||
properties and methods). It is violation of `ISP` from `SOLID`
|
||||
- Examples of not hidden details:
|
||||
- component contains searchable list and exposes search string assuming that
|
||||
filtering will be done externally
|
||||
|
||||
- Favor composition over parameterization and inheritance.
|
||||
|
||||
Overly parameterized components are fragile and not easy to maintain. Big set
|
||||
of switches, altering the appearance and behavior of the component, introduces
|
||||
a lot of conditional statements in the internal implementation. Finally,
|
||||
it is difficult to assure that all combinations are valid or specify which ones
|
||||
are valid and desired.
|
||||
|
||||
- Do not make assumptions about the context in which the component will be used.
|
||||
|
||||
```qml
|
||||
Item {
|
||||
anchors.fill: parent // Bad, will lead to warnings when used in a Layout
|
||||
// ...
|
||||
}
|
||||
```
|
||||
|
||||
In most cases, it is a good idea to use `Control` (or more specialized component)
|
||||
as the base component.
|
||||
|
||||
```qml
|
||||
Control {
|
||||
contentItem: ColumnLayout {
|
||||
// ...
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- Action signals (e.g. click on delegate) should not provide metadata from the
|
||||
model. Only `index` or unique `key` should be an argument. The necessary
|
||||
metadata should be fetched on the signal receiver side.
|
||||
|
||||
```qml
|
||||
signal collectibleClicked(int chainId, string contractAddress,
|
||||
string tokenId, string uid, int tokenType) // BAD
|
||||
signal collectibleClicked(var collectible) // BAD
|
||||
|
||||
signal collectibleClicked(string key) // OK
|
||||
```
|
||||
|
||||
Otherwise, the component unnecessarily bypasses some values from the model.
|
||||
Callers may also want to access roles which are not used for displaying.
|
||||
It leads to creating unnecessary requirements for the input model.
|
||||
|
||||
- use consistently `key` as a unique identifier of the model, even if there are
|
||||
other roles with unique values.
|
||||
|
||||
On the UI side, `key` should be used only for identification, with no other
|
||||
assumptions and usage. Thanks to that, content of that role can be freely
|
||||
changed on the backend with no implications to the UI side. Even if `address`
|
||||
is unique, a separate `key` role should be used (providing the same content as
|
||||
`address` role).
|
||||
|
||||
Other good name for that role - `id` - does not fit well in qml environment as
|
||||
it is reserved keyword. Defining such role with `ListElement` in tests or
|
||||
Storybook would be not possible.
|
||||
|
||||
- `model.rowCount()` should not be used in bindings.
|
||||
|
||||
This call provides count only once when it is called, and the expression will not
|
||||
be re-evaluated when count is changed. Solution is to use the attached property
|
||||
`ModelCount` (`model.ModelCount.count`) context property instead. However,
|
||||
in signal handling, using `rowCount()` is fully correct and preferred over `ModelCount`
|
||||
attached property (because it does not create any additional attached object unnecesarily).
|
||||
|
||||
- `model.count` should not be used on models taken from outside as a dependency.
|
||||
|
||||
`count` property is not a part of the `QAbstractItemModel` interface. Proxy
|
||||
models and backend models may not have that property defined.
|
||||
|
||||
- Objects holding the whole model's row should not be used in bindings.
|
||||
|
||||
```qml
|
||||
readonly property var selectedAccount: ModelUtils.get(store.accounts, 0) // BAD
|
||||
```
|
||||
|
||||
Such objects may be deleted at any time, e.g. because of model reset. The
|
||||
expression will not be automatically re-evaluated to take the current first item.
|
||||
|
||||
- Avoid `QML`'s dynamic scoping. Refer for details
|
||||
[here](https://doc.qt.io/qt-5/qtqml-documents-scope.html#component-instance-hierarchy).
|
||||
|
||||
- Follow [C++ Core Guidelines](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines)
|
||||
in C++ code.
|
||||
|
||||
## Stores
|
||||
|
||||
Store are an intermediate layer between the backend and the UI components.
|
||||
They expose backend's functionality grouped by domain (e.g. `TransactionStore`, `AssetsStore`,
|
||||
`CommunitiesStore`), without following the UI layouts, where one view can interact with multiple
|
||||
domains (e.g. communities and wallet's tokens). In other words, changes in the UI should
|
||||
not result in changes in the store layer.
|
||||
|
||||
> [!NOTE]
|
||||
> The store layer will undergo significant structural changes, including removing singletons,
|
||||
> moving some code elsewhere, deduplication to expose a given backend functionality in only one
|
||||
> store, and leaving at most one `RootStore`. For this reason, the following points are more
|
||||
> about the target state than the current state.
|
||||
|
||||
- Stores are a thin wrapper over context properties exposed from the backend, no
|
||||
additional logic should be there like e.g. data transformations done by proxy models.
|
||||
|
||||
Stores are cut-off line in tests and `Storybook pages`. When additional logic
|
||||
is put there, it is not available in unit tests and `Storybook`. It leads to
|
||||
more complicated mocking and code duplication (same logic repeated in real
|
||||
stores and mocked ones).
|
||||
|
||||
```qml
|
||||
// WalletAssetsStore.qml
|
||||
|
||||
QtObject {
|
||||
property LeftJoinModel groupedAccountAssetsModel: LeftJoinModel {
|
||||
leftModel: root.baseGroupedAccountAssetModel
|
||||
rightModel: _jointTokensBySymbolModel
|
||||
joinRole: "tokensKey"
|
||||
} // Bad - transformation should be kept in adaptor object,
|
||||
// only basic models should be exposed from store
|
||||
}
|
||||
```
|
||||
|
||||
- Stores completely hide the backend's context properties and not expose them
|
||||
directly.
|
||||
|
||||
It makes the contract between backend and frontend clear already from the
|
||||
perspective of the store. It should be enough for a UI developer to rely on
|
||||
the store's API without inspection of the nim code in order to discover what
|
||||
methods and properties are exposed from objects directly exposed to the UI
|
||||
from the backend via store.
|
||||
|
||||
```qml
|
||||
// WalletStore.qml
|
||||
|
||||
QtObject {
|
||||
property var networksModuleInst: networksModule // BAD
|
||||
}
|
||||
```
|
||||
|
||||
```qml
|
||||
// WalletStore.qml
|
||||
|
||||
QtObject {
|
||||
id: root
|
||||
|
||||
readonly property bool isGoerliEnabled: networksModule.isGoerliEnabled
|
||||
|
||||
signal chainIdFetchedForUrl(string url, int chainId)
|
||||
|
||||
function foo() {
|
||||
return networksModule.foo()
|
||||
}
|
||||
|
||||
Component.onCompleted: {
|
||||
networksModule.chainIdFetchedForUrl.connect(root.chainIdFetchedForUrl)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
This additional layer causes that backend changes affect only this part of
|
||||
the UI code, which can be adjusted here even without changing the API of the
|
||||
store itself. It is also clear what exactly is exposed.
|
||||
|
||||
- A single context property injected from the backend should be exposed only
|
||||
once in a single store.
|
||||
|
||||
Bad:
|
||||
|
||||
```qml
|
||||
// SwapStore.qml
|
||||
QtObject {
|
||||
readonly property var flatNetworks: networksModule.flatNetworks
|
||||
}
|
||||
|
||||
// CommunityTokensStore.qml
|
||||
QtObject {
|
||||
readonly property var flatNetworks: networksModule.flatNetworks
|
||||
}
|
||||
```
|
||||
|
||||
Stores represent the state of the application but they are not aware how that
|
||||
state is rendered and interacted by UI components. There is no rule that a
|
||||
single component must take a single store with everything exposed what is
|
||||
needed for that component. On the contrary, the UI component should take all
|
||||
the stores (assuming that stores do not duplicate exposition) it needs as a
|
||||
dependency. Moving on, in most cases a UI component should only take
|
||||
the models/properties it needs and not the entire store.
|
||||
|
||||
- Stores are not singletons (if an existing singleton store needs to be used,
|
||||
still can be taken by a component as an explicit dependency).
|
||||
|
||||
When stores are singleton, UI components can access backend from arbitrary
|
||||
places, in an implicit way. Dependencies are harder to track, APIs are not
|
||||
clear as they could be and it causes other problems covered in the `general` section.
|
||||
|
||||
```qml
|
||||
isGoerliEnabled: root.rootStore.isGoerliEnabled // OK
|
||||
isGoerliEnabled: WalletStores.RootStore.isGoerliEnabled // Bad
|
||||
```
|
||||
|
||||
- Exposed properties are read-only, state modification is done by methods.
|
||||
|
||||
This approach makes the data flow unidirectional. UI always transforms and
|
||||
renders read-only data. UI requests any changes via methods (also called actions).
|
||||
Those actions may result in updates of one or more read-only properties/models.
|
||||
|
||||
- Stores should be always typed, no need to use `var`.
|
||||
|
||||
Thanks to overriding proper import paths, typed stores are not a problem for
|
||||
tests and Storybook pages. They can be freely mocked, with the type preserved.
|
||||
|
||||
```qml
|
||||
required property TransactionStore store // OK
|
||||
required property var store // Bad
|
||||
```
|
||||
|
||||
## Adaptors
|
||||
|
||||
Adaptors are special type of `QML` components responsible for transforming backend's data.
|
||||
Thanks to isolating data (especially models) transformations in adaptors, views
|
||||
take possibly simple, ready to display data (models and other read-only properties).
|
||||
|
||||
Adaptors are usually a composition of proxy models (from external library
|
||||
`SortFilterProxyModel` and from own library of proxy models: `ObjectProxyModel`,
|
||||
`LeftJoinModel`, `GroupingModel`, `ConcatModel`, `RenamingProxyModel`,
|
||||
`MovableModel`, `WritableModel`).
|
||||
|
||||
Diagram depicting custom proxy models can be found [here](https://miro.com/app/board/uXjVKTuFYOU=/)
|
||||
|
||||
- Adaptors are data-oriented and not tightly coupled to a specific view. Adaptors' names
|
||||
should reflect the transformation they do instead of following the naming of the UI component
|
||||
consuming them.
|
||||
|
||||
- API of adaptors, similarly as for UI components, should be possibly explicit
|
||||
and simple. Passing the whole store is usually not a good idea because it is
|
||||
not clear which part of the store's API is really used by the adaptor. Taking
|
||||
plain models with well-specified expected roles is usually a better idea.
|
||||
|
||||
- Output of one adaptor (usually model) can be an input of another adaptor. This
|
||||
structure should intuitively reflect the branches in the data flow in the application.
|
||||
|
||||
## Storybook
|
||||
|
||||
### Features
|
||||
|
||||
Storybook is an internal tool that supports rapid development of components in
|
||||
isolation, outside the application. It is also a kind of catalog of components from
|
||||
which the application is built.
|
||||
|
||||
It offers a number of functionalities that improve development:
|
||||
|
||||
- hot reloading
|
||||
- import paths overloading and `stubs` to isolate from the backend
|
||||
- integrated test runner
|
||||
- figma integration
|
||||
- built-in visual components inspector
|
||||
- pages organized in groups
|
||||
- bunch of dedicated components like `GenericListView` to create pages quickly
|
||||
|
||||
> [!NOTE]
|
||||
> The `StatusQ` library also contains this type of utility application, called `Sandbox`,
|
||||
> which is currently deprecated as it offers less functionality.
|
||||
|
||||
### Running
|
||||
|
||||
`Storybook` is a `cmake` project and can be easily opened, compiled and run in
|
||||
`QtCreator` from the `storybook/CMakeLists.txt` file.
|
||||
|
||||
### Good practices
|
||||
|
||||
- Single `page` instantiates a single component and only necessary mocks/test
|
||||
data and auxiliary controls interacting with component's API.
|
||||
|
||||
It makes the pages simple, easy to maintain and directly presenting the API of
|
||||
a given component.
|
||||
|
||||
- Storybook pages are valuable for all types of components - both basic ones
|
||||
(delegates, buttons) and more complex ones, doing complex flows like e.g.
|
||||
funds transfer.
|
||||
|
||||
- Storybook (and unit tests) has a mechanism of so-called `stubs` to replace
|
||||
real stores with empty objects, but in a way preserving types. It is needed
|
||||
because in Storybook and unit tests backend's context properties used in real
|
||||
stores are not available. Store's stubs are intended to be empty QtObjects,
|
||||
actual mocking should be done within a page.
|
||||
|
||||
This approach allows making the dependencies of the component truly visible
|
||||
and explicit in a Storybook page. An alternative approach would be to
|
||||
implement single, shared mocks for every store and share the among pages. But
|
||||
it leads to situation that given mock provides much more than is needed by a
|
||||
single UI component. As a consequence, it is not clear on which part of the
|
||||
store given component depends on. A separate thing is that UI components
|
||||
should not depend on the store at all unless it is really necessary (only top
|
||||
level components covering complex flows).
|
||||
|
||||
- A component in Storybook should be functional.
|
||||
|
||||
The state of a component's Storybook page quite accurately reflects the state
|
||||
of the component. Components with a well-designed API and no unnecessary
|
||||
dependencies are easy to instantiate in isolation in Storybook. Conversely,
|
||||
overly complex components are a nightmare in Storybook.
|
||||
|
||||
- Use identified modules to import stores (using unquoted identifiers). It is
|
||||
required for the stubs mechanism to work properly.
|
||||
|
||||
In practice imports like `import AppLayouts.Wallet.stores 1.0 as WalletStores`
|
||||
should be used instead of `import "./stores" as WalletStores`. Second version is
|
||||
relative import from the file system. As a consequence, the mechanism for
|
||||
overriding import paths for tests and Storybook's pages will not work.
|
||||
|
||||
## Code Style
|
||||
|
||||
- QML code should be in-line with
|
||||
[QML Coding Conventions](https://doc.qt.io/qt-5/qml-codingconventions.html).
|
||||
- Top level component `id` should be always `root`.
|
||||
- Private properties should be hidden in a `d` object (`QtObject { id: d }`, or
|
||||
`QObject { id: d }`).
|
||||
- `objectName` property should not be specified for the top level component in
|
||||
a given file.
|
||||
|
||||
Component may be used in various context and usually should have different
|
||||
name in every context to disambiguate when doing lookup in unit tests, squish
|
||||
tests or monitoring tool.
|
||||
- `qmldir`: entries should be sorted (`Alt+Shift+S` on selection in `QtCreator`).
|
||||
- Comments and documentation should cover parts which are not obvious, can be
|
||||
skipped where intention is clear.
|
||||
- A declarative implementation should be favoured over imperative code in QML
|
||||
in most cases.
|
||||
- Use curly brackets to make complex expressions easier to read:
|
||||
|
||||
```qml
|
||||
readonly property bool errorMode: popup.isLoading || !recipientInputLoader.ready ?
|
||||
false : errorType !== Constants.NoError
|
||||
|| networkSelector.errorMode
|
||||
|| !(amountToSendInput.inputNumberValid
|
||||
|| d.isCollectiblesTransfer) // Hard to read
|
||||
```
|
||||
|
||||
```qml
|
||||
readonly property bool errorMode: { // Easier to read
|
||||
if (popup.isLoading || !recipientInputLoader.ready)
|
||||
return false
|
||||
|
||||
return errorType !== Constants.NoError
|
||||
|| networkSelector.errorMode
|
||||
|| !(amountToSend.ready || d.isCollectiblesTransfer)
|
||||
}
|
||||
```
|
||||
|
||||
## Other
|
||||
|
||||
- Cryptocurrency balances should be always handled as a big integer strings and
|
||||
converted to localized human-friendly form only for displaying. Some basic rationale is
|
||||
provided in [this ticket](https://github.com/status-im/status-desktop/issues/11376).
|
||||
|
||||
## Useful links
|
||||
|
||||
- [Qt Quick Layouts Overview](https://doc.qt.io/qt-6/qtquicklayouts-overview.html)
|
||||
- [Customizing Qt Quick Controls](https://doc.qt.io/qt-6/qtquickcontrols-customize.html)
|
||||
- [Keyboard Focus in Qt Quick](https://doc.qt.io/qt-6/qtquick-input-focus.html)
|
||||
- [Important Concepts In Qt Quick - Positioning
|
||||
](https://doc.qt.io/qt-6/qtquick-positioning-topic.html)
|
||||
|
|
@ -2,6 +2,8 @@
|
|||
|
||||
[Status](https://status.app/) Desktop client built with [Nim](https://nim-lang.org/) and [Qt](https://www.qt.io/).
|
||||
|
||||
#### [Contributing](CONTRIBUTING.md)
|
||||
|
||||
#### [Our Website](https://status.app/)
|
||||
|
||||
#### [Need Help?](https://status.app/help)
|
||||
|
|
@ -10,4 +12,4 @@
|
|||
|
||||
#### [You have an idea for a cool feature or improvement?](https://status.app/feature-upvote)
|
||||
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -1,96 +0,0 @@
|
|||
# Contributing guidelines
|
||||
The main goal of the following document is to have a `basic set of guidelines` that all contributors should apply when writing code in the `status-desktop` project and specific rules for using the C++ language in order to achieve consistent, maintainable and easy-to-understand code for everyone.
|
||||
|
||||
There are lots of existing and extended guidelines that can be used, as well, as a complement of the ones that will be described below. Here some examples:
|
||||
- [Cpp Core Guidelines](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md)
|
||||
|
||||
## Generic rules
|
||||
|
||||
General software development best practices and / or OPP specific ones:
|
||||
- Write code using a syntax that can make the `code easy to understand` although it requires more writing. The time spent to understand that code by other contributors will be compensated.
|
||||
- Create `self-documented` code by using self-explanatory names wherever possible for classes, methods and variables. It will increase readability and comprehension.
|
||||
- Keep in mind the `SOLID` principles. Your code will become flexible, reusable, scalable and prepared for test-driven development.
|
||||
- Keep `methods body short`. Divide when possible. Recursive calls and nested loops should be minimized.
|
||||
- Use `parenthesis` to resolve ambiguity in complex expressions. Each individual expression should be written in its single parenthesis.
|
||||
- Use the following `arguments order` in method's `signature` (it will force default argument's values to be input ones):
|
||||
- Input / Output.
|
||||
- Output.
|
||||
- Input.
|
||||
- `Initialize` all `variables` when declared.
|
||||
- `Initialize` all class `attributes` in their own class constructor.
|
||||
- Use `m_` syntaxis to define attribute names.
|
||||
- Class and method names should begin with `capital letter` and use `camel case` approach.
|
||||
- Use the following `class / members declaration structure`:
|
||||
- Public methods / members.
|
||||
- Protected methods / members.
|
||||
- Private methods / members.
|
||||
- Code should compile `without warnings`.
|
||||
|
||||
|
||||
## C++ language specific rules
|
||||
- As a general rule, for `smart pointers`, `containers` and `threads`, use `STL for domain` and `Qt specific for controllers` (layer exposed to QML).
|
||||
- Use smart pointers for general dynamic/heap memory management: STL with make_shared and make_unique.
|
||||
- Use Qt’s parent-child relationship and built-in ownership in API where appropriate: QObject-based with QPointer.
|
||||
- Use `const` keyword wherever possible it will build a more robust code. If it is used in variables, the held value cannot be modified; in pointers, the address pointed to cannot be modified (no restrictions for the stored data); in class methods, the method cannot modify any class attribute.
|
||||
- When passing arguments, use `references` instead of pointers as far as possible by using `&`. It will prevent some typical errors such as passing null pointers to methods.
|
||||
- When passing arguments, `avoid passing by value` big size arguments as structures and objects. It will reduce the stack used and it will not be necessary to have properly implemented the copy constructor in all classes.
|
||||
- Use `dynamic allocation` only if needed because it requires much processor time than stack memory allocation. By minimizing its usage for variables and objects will increase efficiency and `reduce` the risk of `memory leaks`.
|
||||
- `Thread’s` usage must be `justified`. Always keep in mind the principle “Keep It Simple”.
|
||||
- The `C++ standard` to be used must be `at least C++17`.
|
||||
- `Singleton’s` usage must be `justified`. They must be just only global settings and specific objects but to use them it will need a consensus with the team.
|
||||
- `.clang-tidy` configuration will be used as the C++ linter and at least the following checkers will be set (it must be included in the CI process):
|
||||
- cppcoreguidelines-*
|
||||
- modernize-*
|
||||
- readibility-*
|
||||
|
||||
## Documentation
|
||||
|
||||
- Use `header files` to document classes and methods by using `doxygen syntax`. More information here: [Doxygen manual](https://www.doxygen.nl/index.html).
|
||||
|
||||
|
||||
```
|
||||
/*
|
||||
* Add here header file description.
|
||||
* \file Example.h
|
||||
* \brief Source code file.
|
||||
*/
|
||||
|
||||
...
|
||||
|
||||
/*
|
||||
* \brief Add here short class description.
|
||||
* This class implements a ...
|
||||
*/
|
||||
class Example {
|
||||
public:
|
||||
/*
|
||||
* \brief Class constructor
|
||||
*/
|
||||
Example();
|
||||
|
||||
/*
|
||||
* \brief Class destructor.
|
||||
*/
|
||||
~Example();
|
||||
|
||||
protected:
|
||||
/*
|
||||
* \brief Add here short method description.
|
||||
* This method ...
|
||||
* \return True if the operation was successfully completed; False otherwise.
|
||||
*/
|
||||
bool Example2();
|
||||
|
||||
private:
|
||||
/*
|
||||
* \brief Add here short method description.
|
||||
* This method...
|
||||
*/
|
||||
Example3(bool bEnabled //!< [in] Enable argument value.
|
||||
);
|
||||
|
||||
...
|
||||
}
|
||||
```
|
||||
|
||||
|
||||
|
|
@ -16,6 +16,9 @@ import StatusQ.Core.Theme 0.1
|
|||
|
||||
The \c StatusScrollView can be used just like a plain ScrollView but without ability to decarate existing Flickable.
|
||||
|
||||
A presenation on using StatusScrollView can be found here:
|
||||
https://docs.google.com/presentation/d/1ZZeg9j2fZMV-iHreu_Wsl1u6D9POH7SlUO78ZXNj-AI
|
||||
|
||||
Simple example of how to use it:
|
||||
|
||||
\qml
|
||||
|
|
|
|||
|
|
@ -1,313 +0,0 @@
|
|||
# Contributing guidelines
|
||||
|
||||
## Generic Reusable Component Design
|
||||
|
||||
Define the component's purpose. Narrow down the scope as much as possible.
|
||||
|
||||
Avoid assumptions about its parent/owner
|
||||
|
||||
- See [broken behavior example](./code-examples.md#Dont-assume-parents-context-bad-example)
|
||||
- See [well-behaved behavior example](./code-examples.md#Give-user-choice)
|
||||
|
||||
Define and make clear the interface/contract that defines requirements.
|
||||
|
||||
To simplify maintenance consider the following options for implementing custom controls, generic containers or views:
|
||||
|
||||
- Use QML2's customization mechanics in place if the change is simple: [Check docs for examples](https://doc.qt.io/qt-6/qtquickcontrols2-customize.html)
|
||||
- If the control should be generic or the changes are extensive and the design maps to a QML control, use QML2's customization mechanisms in a generic QML control: [Check docs for examples](https://doc.qt.io/qt-6/qtquickcontrols2-customize.html) instead of starting from scratch.
|
||||
- Use Qt controls instead of defining our own. Defining and implementing a complex control from scratch should be the last resort and there should be an excellent reason to do that.
|
||||
- For keyboard input item, ensure focus properties are set; see [Qt docs](https://doc.qt.io/qt-6/qtquick-input-focus.html)
|
||||
|
||||
When unsure, check Qt's excellent documentation.
|
||||
|
||||
- For main components/controls, there is a good overview that is worth revisiting from time to time
|
||||
- Functions/Properties usually have a short on-point description
|
||||
- In `QtCreator` you can quickly open the doc panel using `F1` key while having a cursor on component or function/property and recheck its invariants, pre and post conditions
|
||||
|
||||
Consider that design follows user-friendlier principles.
|
||||
|
||||
- Have commonly used items at hand
|
||||
- Have transition if possible
|
||||
- Use Qt's property animation for states
|
||||
- Avoid often and radical size changes to items in views/layouts. Radical size change confuses users.
|
||||
- If the content is not available, consider having placeholders using the estimated size
|
||||
(delegates, dummy items)
|
||||
|
||||
Have the base control as the component's root. This way, control inherits all the interface and reduce the code.
|
||||
If it doesn't map to an existing one, use a base control like `Item`.
|
||||
|
||||
- Don't use layouts or positioners as base for controls, they don't have the same behavior when used in layouts/containers
|
||||
- Layouts have `fillWidth`/`fillHeight` as true by default and they will be extended. Controls don't and they will follow implicit sizes
|
||||
|
||||
## QML well-behaved components checklist
|
||||
|
||||
### Positioning and sizing
|
||||
|
||||
[Positioners and Layouts In QML](https://doc.qt.io/qt-6/qtquick-usecase-layouts.html)
|
||||
|
||||
- Define size hints appropriately. Define implicit properties (`impicitWidth` and `implicitHeight`)
|
||||
|
||||
- They are used to break polishing binding loops for adaptive control
|
||||
- All the containers use the implicit size for deriving the initial size (Layouts, Popups, Delegates, GridView, ListView ...). Size hints combined with resize adaptability are the appropriate way to have reactive controls.
|
||||
- For complex controls, look if layouts are the choice as the main position driver. [Layouts](###Layouts)
|
||||
|
||||
```qml
|
||||
Item {
|
||||
id: root
|
||||
implicitWidth: mainLayout.implicitWidth
|
||||
implicitHeight: mainLayout.implicitHeight
|
||||
RowLayout { // Column, Grid
|
||||
id: mainLayout
|
||||
// ...
|
||||
}
|
||||
```
|
||||
|
||||
- Adapt to the requested size if control can scale or it make sense to be extensible. For sizes bigger than content follow QML way for similar controls. e.g. Text leaves empty space around content
|
||||
- If the control is not adaptable and makes sense only in its full size, define default sizes and make it work by default with positioners (`Row`, `Column`, `Grid`).
|
||||
- [Item Positioners in QML](https://doc.qt.io/qt-5/qtquick-positioning-layouts.html)
|
||||
- Don't mix hints with sizes, it will create binding loops
|
||||
|
||||
### Quality
|
||||
|
||||
Pixel miss-alignment for sensitive elements like icons and images with straight and thin lines can degrade the quality perceptibly.
|
||||
|
||||
Dividing sizes and coordinates are the usual root cause of pixel miss-alignment. Correct subpixel division by rounding the result of the operation to a fixed value, e.g. `x: (width/2).toFixed()`
|
||||
|
||||
- Example of pixel miss-alignment. The right control has a position offset of `Qt.pos(0.7, 0.4)` therefore, the image interpolation generates a blurred rendering.
|
||||
- 
|
||||
|
||||
## Topics
|
||||
|
||||
### QML Coding Conventions
|
||||
|
||||
[Follow Qt's way if appropriate](https://doc.qt.io/qt-6/qml-codingconventions.html)
|
||||
|
||||
### Layouts
|
||||
|
||||
[Qt Quick Layouts Overview](https://doc.qt.io/qt-6/qtquicklayouts-overview.html)
|
||||
|
||||
Hierarchically sources hints from children. Implicit properties will have the recommended aggregated size.
|
||||
|
||||
Layouts as children of other layouts have `fillWidth` and `fillHeight` enabled; controls don't.
|
||||
|
||||
Use `Layout.preferredWidth` and `Layout.preferredHeight` attached properties to overwrite item's `implicitWidth` if they are not appropriate to the design.
|
||||
|
||||
### Scope
|
||||
|
||||
Follow [Qt's recommendations](https://doc.qt.io/qt-5/qtqml-documents-scope.html) if appropriate
|
||||
|
||||
- Avoid dynamic scoping in out-of-line components, see more: https://doc.qt.io/qt-5/qtqml-documents-scope.html#component-instance-hierarchy
|
||||
|
||||
- Example
|
||||
|
||||
```qml
|
||||
Item {
|
||||
// Probably won't work as intended. If another `testComp` instance is defined in QML's document hierarchy model and has a `testProp` property, that will be sourced instead
|
||||
property bool booleanProp: testComp.testProp
|
||||
// Same behavior if the TestComponent is defined in a file TestComponent.qml
|
||||
component TestComponent: Item {
|
||||
id: testComp
|
||||
property bool testProp: false
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- Component `id`s are not accessible outside the component's scope. If required, the component can expose the instance through properties binding. E.g. `readonly property ComponentType exposedComponent: privateInstanceId`
|
||||
|
||||
- If in doubt, explicitly use an instance variable to access properties
|
||||
- If the scope is clear and there is no ambiguity, use the property directly for readability
|
||||
|
||||
### Popups content sizing
|
||||
|
||||
As stated in Qt documentation on Popup content sizing:
|
||||
|
||||
1. `Popup` size is as calculated internally as `contentItem.implicitSize + paddings`
|
||||
2. The size of `contentItem` is managed by the popup to fit the Popup and paddings
|
||||
|
||||
It's important to understand that `Popup.contentItem` is an internal invisible Item. Any user items will be parented to the `contentItem`, not the `Popup` itself.
|
||||
|
||||
Knowing this, we make a simple rule to choose one of 2 ways for popup content sizing:
|
||||
|
||||
1. Replace the `contentItem` with your item, but don't explicitly anchor or bind it's sizes, because the size of `contentItem` is managed by the popup.
|
||||
|
||||
```qml
|
||||
Popup {
|
||||
contentItem: ColumnLayout {
|
||||
// no anchoring or binding
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
2. Don't touch the `contentItem` property, but create a single child to the popup and **anchor it to `parent`**.
|
||||
|
||||
```qml
|
||||
Popup {
|
||||
ColumnLayout {
|
||||
anchors.fill: parent // parent is contentItem, not Popup
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### StatusScrollView
|
||||
|
||||
> A presenation on using `StatusScrollView` can be found [here](https://docs.google.com/presentation/d/1ZZeg9j2fZMV-iHreu_Wsl1u6D9POH7SlUO78ZXNj-AI).
|
||||
|
||||
#### Basic usage
|
||||
|
||||
```qml
|
||||
StatusScrollView {
|
||||
anchors.fill: parent // Give the SceollView some size
|
||||
|
||||
// - No need to specify contentWidth and contentHeight.
|
||||
// For a single child item it will be calculated automatically from implicit size of the item.
|
||||
// The item must have implicit size specified. Not width/height.
|
||||
|
||||
Image {
|
||||
source: "https://placekitten.com/400/600"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
#### Filling width
|
||||
|
||||
```qml
|
||||
StatusScrollView {
|
||||
id: scrollView
|
||||
anchors.fill: parent
|
||||
contentWidth: availableWidth // Bind ScrollView.contentWidth to availableWidth
|
||||
|
||||
ColumnLayout {
|
||||
width: scrollView.availableWidth // Bind content width to availableWidth
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
#### In a popup
|
||||
|
||||
1. Use when `StatusScrollView` is the _only direct child_ of popup's `contentItem`.
|
||||
2. If you have other items outside the scroll view, you will have to manually apply paddings to them as well.
|
||||
|
||||
```qml
|
||||
StatusModal {
|
||||
padding: 0 // Use paddings of StatusScrollView
|
||||
|
||||
StatusScrolLView {
|
||||
id: scrollView
|
||||
|
||||
anchors.fill: parent
|
||||
implicitWidth: 400
|
||||
contentWidth: availableWidth
|
||||
padding: 16 // Use padding of StatusScrollView, not StatusModal
|
||||
|
||||
Text {
|
||||
width: scrollView.availableWidth
|
||||
wrapMode: Text.WrapAnywhere
|
||||
text: "long text here"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
#### Deep in a popup
|
||||
|
||||
1. Use when `StatusScrollView` or `StatusListView` is not a direct child of `contentItem`, or it's not the only child.
|
||||
2. All popup contents are aligned to given paddings, but the scroll bar doesn't overlay the content and is positioned right inside the padding.
|
||||
|
||||
```qml
|
||||
StatusModal {
|
||||
padding: 16 // Use popup paddings
|
||||
|
||||
ColumnLayout {
|
||||
anchors.fill: parent
|
||||
|
||||
Text {
|
||||
Layout.fillWidth: true
|
||||
text: "This header is fixed and not scrollable"
|
||||
}
|
||||
|
||||
Item {
|
||||
id: scrollViewWrapper
|
||||
|
||||
Layout.fillWidth: true
|
||||
Layout.fillHeight: true
|
||||
implicitWidth: scrollView.implicitWidth
|
||||
implicitHeight: scrollView.implicitHeight
|
||||
|
||||
StatusScrollView {
|
||||
id: scrollView
|
||||
|
||||
anchors.fill: parent
|
||||
contentWidth: availableWidth
|
||||
padding: 0 // Use popup paddings
|
||||
|
||||
// Detach scrollbar
|
||||
ScrollBar.vertical: StatusScrollBar {
|
||||
parent: scrollViewWrapper
|
||||
anchors.top: scrollView.top
|
||||
anchors.bottom: scrollView.bottom
|
||||
anchors.left: scrollView.right
|
||||
anchors.leftMargin: 1
|
||||
}
|
||||
|
||||
Text {
|
||||
width: scrollView.availableWidth
|
||||
wrapMode: Text.WrapAnywhere
|
||||
text: "long scrollable text here"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Testing
|
||||
|
||||
Test in isolation
|
||||
|
||||
- Use `qmlproject`s and `qml` for quick complex scenarios
|
||||
- Use [`QtQuick.TestCase`](https://doc.qt.io/qt-5/qtquicktest-index.html)
|
||||
|
||||
Integration tests
|
||||
|
||||
- Use sandbox test app
|
||||
- Use QML `qmlproject`s and `qmlscene` for quick debugging with layouts
|
||||
|
||||
Try scenarios
|
||||
|
||||
- Embed in Layouts with different properties: fixed size, min-max, not size set
|
||||
|
||||
```qml
|
||||
Window {
|
||||
width: mainLayout.implicitWidth
|
||||
heigh: mainLayout.implicitHeight
|
||||
|
||||
GridLayout {
|
||||
id: mainLayout
|
||||
rows: 3
|
||||
column: 3
|
||||
|
||||
anchor.fill: parent
|
||||
Label { text: "Fixed width" }
|
||||
TestControl {
|
||||
Layout.preferredWidth: 100
|
||||
Layout.preferredHeight: 100
|
||||
}
|
||||
Label { text: "Fill space" }
|
||||
TestControl {
|
||||
Layout.fillWidth: true
|
||||
Layout.fillHeight: true
|
||||
}
|
||||
Label { text: "Width range" }
|
||||
TestControl {
|
||||
Layout.minWidth: 50
|
||||
Layout.maxWidth: 150
|
||||
}
|
||||
TestControl {}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- Resize window to check the behavior for each case
|
||||
- Visually validate that each control is behaving as expected
|
||||
- Add controls with different properties that affect control size behavior
|
||||
|
|
@ -17,6 +17,7 @@ SOURCES += $$files("$$PWD/*qmldir", true)
|
|||
SOURCES += $$files("$$PWD/*.qml", true)
|
||||
SOURCES += $$files("$$PWD/*.js", true)
|
||||
SOURCES += $$files("$$PWD/../monitoring/*.qml", true)
|
||||
SOURCES += $$files("$$PWD/../*.md", false)
|
||||
}
|
||||
|
||||
# Other *.ts files will be provided by Lokalise platform
|
||||
|
|
|
|||
Loading…
Reference in New Issue