Conversation
src/declarativesplitter.cpp
Outdated
|
|
||
| This file is part of DeclarativeWidgets, library and tools for creating QtWidget UIs with QML. | ||
|
|
||
| Copyright (C) 2013-2017 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com |
There was a problem hiding this comment.
Could you fix the dates in the copyright notices please?
|
In principle I don't see a problem with having a stretch attached property as the original API is only a convenience on changing the size policy (something you couldn't do in DeclarativeWidgets when this patch was created). This change looks like it would be a good candidate for an addition to the layouts unit test. Whilst not strictly a layout, there are already functions there to compare the geometry of widgets created with imperative Qt Widget and DeclarativeWidgets. Could you add a test case please, comparing layouts created using splitters please? |
|
Absolutely, thanks for the feedback. I will look into this later this week. |
krake-kdab
left a comment
There was a problem hiding this comment.
Isn't that now already covered by the new support for size policy?
src/splitterwidgetcontainer.cpp
Outdated
|
|
||
| void SplitterWidgetContainer::addWidget(QWidget *widget) | ||
| { | ||
| QObject *attachedProperties = qmlAttachedPropertiesObject<QSplitter>(widget, false); |
There was a problem hiding this comment.
that should probably be DeclarativeSplitter as the template argument
0x6e
left a comment
There was a problem hiding this comment.
As it stands, the attached property is only going to update the widget stretch when the widget is added to the Splitter. Would it not be better to have the attached property directly read and write from the widget size policy?
|
Okay, I think I have fixed everything. Also, please note that there is some inconsistency in the formatting and the dates in the copyright headers. Some parts of the project use 2 space indent, while others (like the test cases) use 4 space indent. Also, some headers simply have "2017" as date, mainly the test parts of the code, while others have "2013-2017". |
Great!
Yes, the original 2 space indent came from being a "private playground project" from before there was a Qt indentation standard.
I would say just the current year for new files |
krake-kdab
left a comment
There was a problem hiding this comment.
Looks like DeclarativeSplitter is no longer needed
|
You're right, I have removed it now. |
Add support for Splitter. I am not entirely sure about putting stretch as an attached property of Splitter, since it really modifies the SizePolicy of the widget, but it makes for a nice and consistent (with HBoxLayout) API.