dfaure added a comment.
The unittest in this commit appears to break in CI.
https://build.kde.org/job/Frameworks/view/Platform%20-%20SUSEQt5.12/job/kirigami/job/kf5-qt5%20SUSEQt5.12/417/testReport/junit/projectroot.home.jenkins.workspace.Frameworks.kirigami.kf5-qt5_SUSEQt512/autotests/tst
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R169:1d5398acecaa: Add PageRouter component (authored by
cblack).
CHANGED PRIOR TO COMMIT
https://phabricator.kde.org/D28
cblack updated this revision to Diff 80282.
cblack marked an inline comment as done.
cblack added a comment.
Use more descriptive name
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=80275&id=80282
BRANCH
arcpatch-D28383
REVISION DETAIL
https
mart accepted this revision.
mart added inline comments.
INLINE COMMENTS
> pagerouter.cpp:217
> +auto incomingRoutes = parseRoutes(route);
> +QList mut;
> +
more descriptive name
REPOSITORY
R169 Kirigami
REVISION DETAIL
https://phabricator.kde.org/D28383
To: cblack, #kirigami, mar
cblack updated this revision to Diff 80275.
cblack added a comment.
Fix faulty navigateToRoute
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=80223&id=80275
BRANCH
arcpatch-D28383
REVISION DETAIL
https://phabricator.kde.org/D28383
AFFECTED
cblack updated this revision to Diff 80223.
cblack marked 3 inline comments as done.
cblack added a comment.
Address feedback
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=79874&id=80223
BRANCH
arcpatch-D28383
REVISION DETAIL
https://phabri
mart added inline comments.
INLINE COMMENTS
> tst_pagerouter.qml:56
> +id: router
> +initialRoute: "/home"
> +columnView: root.columnView
I find all the "/" in the examples confusing, not sure is a good naming
convention (and works prefectly without having that /, and i
cblack updated this revision to Diff 79874.
cblack added a comment.
Add tests
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=79870&id=79874
BRANCH
cblack/pagerouter
REVISION DETAIL
https://phabricator.kde.org/D28383
AFFECTED FILES
Mainpag
cblack updated this revision to Diff 79870.
cblack added a comment.
Add currentRoutes function
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=79785&id=79870
BRANCH
cblack/pagerouter
REVISION DETAIL
https://phabricator.kde.org/D28383
AFFECTE
cblack updated this revision to Diff 79785.
cblack added a comment.
Add bringToView and isCurrent
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=79774&id=79785
BRANCH
cblack/pagerouter
REVISION DETAIL
https://phabricator.kde.org/D28383
AFFE
cblack updated this revision to Diff 79774.
cblack added a comment.
Fix bad image text
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=79748&id=79774
BRANCH
cblack/pagerouter
REVISION DETAIL
https://phabricator.kde.org/D28383
AFFECTED FILES
cblack updated this revision to Diff 79748.
cblack added a comment.
Improve documentation
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=79746&id=79748
BRANCH
cblack/pagerouter
REVISION DETAIL
https://phabricator.kde.org/D28383
AFFECTED FIL
cblack updated this revision to Diff 79746.
cblack added a comment.
Address feedback (both human and compiler), improve examples, port to
QQmlParserStatus
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=79745&id=79746
BRANCH
cblack/pagerouter
cblack updated this revision to Diff 79745.
cblack added a comment.
Fix Doxygen errors
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=79498&id=79745
BRANCH
cblack/pagerouter
REVISION DETAIL
https://phabricator.kde.org/D28383
AFFECTED FILES
cblack planned changes to this revision.
cblack added a comment.
`@include` not resolving, will fix
REPOSITORY
R169 Kirigami
REVISION DETAIL
https://phabricator.kde.org/D28383
To: cblack, #kirigami, mart, davidedmundson
Cc: ahiemstra, davidedmundson, plasma-devel, fbampaloukas, GB_2, dom
cblack added inline comments.
INLINE COMMENTS
> mart wrote in pagerouter.h:12
> is this still needed for routes that are a composition of PageRoute objects
> like /path/to/some/thing?
This is an internal struct for keeping track of internal state and to have a
C++ representation of a a QJSValu
cblack updated this revision to Diff 79498.
cblack marked 4 inline comments as done.
cblack added a comment.
Better documentation comment
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=79320&id=79498
BRANCH
cblack/pagerouter
REVISION DETAIL
mart requested changes to this revision.
mart added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> pagerouter.h:12
> +
> +struct ParsedRoute {
> +QString name;
is this still needed for routes that are a composition of PageRoute objects
like /path/to/some/t
cblack updated this revision to Diff 79320.
cblack marked an inline comment as done.
cblack added a comment.
Add documentation
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=79082&id=79320
BRANCH
cblack/pagerouter
REVISION DETAIL
https://pha
mart added inline comments.
INLINE COMMENTS
> PageRow.qml:60
> +// Private handle to columnView.
> +property alias _columnView: columnView
> +
if is a publicly accessible property, it should be apublic and documented
REPOSITORY
R169 Kirigami
REVISION DETAIL
https://phabricator.kde.
cblack updated this revision to Diff 79082.
cblack marked an inline comment as done.
cblack added a comment.
Address more feedback
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=79080&id=79082
BRANCH
cblack/pagerouter
REVISION DETAIL
https:/
cblack added inline comments.
INLINE COMMENTS
> ahiemstra wrote in pagerouter.h:308
> If you have routes as objects you could have an "active" property on the
> route, removing the need for this invokable.
Not really, the point of this property is to query whether a full URI is on the
stack. T
cblack updated this revision to Diff 79080.
cblack marked 5 inline comments as done.
cblack added a comment.
Address more feedback
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=79073&id=79080
BRANCH
cblack/pagerouter
REVISION DETAIL
https:/
cblack updated this revision to Diff 79073.
cblack marked 2 inline comments as done.
cblack added a comment.
Consume ColumnView instead of PageRow
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=78948&id=79073
BRANCH
cblack/pagerouter
REVISION
ahiemstra added inline comments.
INLINE COMMENTS
> mart wrote in pagerouter.h:84
> 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"
> co
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::i
cblack updated this revision to Diff 78948.
cblack added a comment.
Add more documentation comments
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=78946&id=78948
BRANCH
cblack/pagerouter
REVISION DETAIL
https://phabricator.kde.org/D28383
AF
cblack updated this revision to Diff 78946.
cblack added a comment.
Get arc to properly diff
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=78927&id=78946
BRANCH
cblack/pagerouter
REVISION DETAIL
https://phabricator.kde.org/D28383
AFFECTED
cblack updated this revision to Diff 78927.
cblack added a comment.
Add caching
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=78747&id=78927
BRANCH
cblack/pagerouter
REVISION DETAIL
https://phabricator.kde.org/D28383
AFFECTED FILES
src/k
cblack updated this revision to Diff 78747.
cblack added a comment.
Get arc to deal with multiple commits properly
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=78746&id=78747
BRANCH
cblack/pagerouter
REVISION DETAIL
https://phabricator.kde
cblack updated this revision to Diff 78746.
cblack added a comment.
Add documentation comments
REPOSITORY
R169 Kirigami
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28383?vs=78744&id=78746
BRANCH
cblack/pagerouter
REVISION DETAIL
https://phabricator.kde.org/D28383
AFFECTE
davidedmundson requested changes to this revision.
davidedmundson added a comment.
This revision now requires changes to proceed.
This needs way more description and explanation of the problem it's trying to
solve.
REPOSITORY
R169 Kirigami
REVISION DETAIL
https://phabricator.kde.org/D283
cblack created this revision.
cblack added reviewers: Kirigami, mart.
Herald added a project: Kirigami.
Herald added a subscriber: plasma-devel.
cblack requested review of this revision.
REVISION SUMMARY
PageRouter is a component that manages named routes of pages.
REPOSITORY
R169 Kirigami
B
33 matches
Mail list logo
