mart requested changes to this revision.
mart added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> pagerouter.cpp:154
> +
> +void PageRouter::setRoutes(QJSValue routes)
> +{
should add parsing and validation here
> pagerouter.cpp:199
> +{
> + QMetaObject::invokeMethod(m_pageRow, "pop");
> +}
pagerow.pop will pop the *current* page and everything on top of it, so you
can't rely on its behavior, it should really use its internal columnview
directly (of which you have c++ access there)
> pagerouter.h:55
> + * }
> + * routes: {
> + * "/home": homeComponent,
add an example route of a/more/complex/path
if i have /home/login, will it always be the same component as just /login? is
not clear here
> pagerouter.h:84
> + * routes: {
> + * "/home": homeComponent
> + * }
how many routes an app would normally be going to have?
this seems really a thing for QQmlListProperty (which then each route must
become a qobject tough)
so routes: [
Route {
name: "home"
component: homeComponent
cache: true
initialProperties: {"name": "value,...}
},
Route {}
]
which kindof becomes similar to pagepoolaction (which i still think they can be
merged)
> pagerouter.h:121
> + */
> + Q_PROPERTY(QQuickItem* pageRow MEMBER m_pageRow)
> +
i'm not sure if i
it hadles directly the creation of the components.. so it really doesn't have a
reason of being a pagerow over columnView which is the internal implementation
of pagerow.
in this paramenter, it should search for a columnview property or something
like that which it can statically check if it's actually a columnview which
will be able to access directly hopefully without needing any invokemethod
> pagerouter.h:168
> + */
> + Q_PROPERTY(bool cachePages MEMBER m_cachePages NOTIFY cachePagesChanged)
> +
this property would be better per page
> pagerouter.h:308
> + */
> + Q_INVOKABLE bool isNavigatedToRoute(QJSValue route);
> +
quite ugly name..
maybe isRouteActive
REPOSITORY
R169 Kirigami
REVISION DETAIL
https://phabricator.kde.org/D28383
To: cblack, #kirigami, mart, davidedmundson
Cc: davidedmundson, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos,
ngraham, apol, ahiemstra, mart
