mart added inline comments.

INLINE COMMENTS

> Page.qml:223
> +     */
> +    property variant icon
> +

for consistency, should be a private/ActionItemGroup.qml (which mimics the 
upstream qqc2 api icon.name/source/width/height/color)

> Page.qml:238
> +     */
> +    property var progress: undefined
> +

again, not convinced we should have this, seems a bit oddly specific

> SwipeNavigator.qml:30
> +     */
> +    property bool largeHeader: false
> +

perhaps any custom heading should be via a custom item (or component?) to put 
as a property (of type qqc2.tabbar)

> SwipeNavigator.qml:38
> +     */
> +    property alias layerStack: stackView
> +

"layers", to have the same api of pageRow

> SwipeNavigator.qml:55
> +        states: [
> +            State {
> +                name: "small"

different tabbars should really depend only on whether it's a mobile device or 
not (and be at bottom if mobile): a very small window on desktop is not a 
mobile app

> SwipeNavigator.qml:66
> +        Kirigami.ActionToolBar {
> +            id: actionToolBar
> +            anchors {

this should be page actions? other actions? what is the exact use case?

> SwipeTabBar.qml:12
> +
> +RowLayout {
> +    id: swipeTabBarRoot

QtQuickControls do have a TabBar control, instead of having a completely custom 
control

> SwipeTabBar.qml:37
> +                    if (index == columnView.currentIndex) {
> +                        return i18nc("Accessibility text for a page tab. 
> Keep the text as concise as possible and don't use a percent sign.", "Current 
> page. Progress: %1 percent.", Math.round(modelData.progress*100))
> +                    } else {

this is kinda outside of the scope of the control.
I guess you have an use case for this, but it shouldn't be a swiss army knife, 
just a tabbar: it's more complicated but at the same time there will always be 
people that miss a feature and will reimplement the whole thing for that.
I think it should just be easy to provide custom items for individual tabs, 
when needed (or just an item to be placed inside a tab, not sure)

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D28873

To: cblack, #kirigami, #vdg
Cc: mart, ndavis, camiloh, plasma-devel, fbampaloukas, GB_2, domson, 
dkardarakos, ngraham, apol, ahiemstra, davidedmundson

Reply via email to