rkflx added a comment.
> Can you commit yourself?
Now I can :) (Thanks for your support with that!)
Will commit once all four patches have been accepted.
REPOSITORY
R236 KWidgetsAddons
BRANCH
arcpatch-D7161
REVISION DETAIL
https://phabricator.kde.org/D7161
To: rkflx,
rkflx added a comment.
Are you sure you are calling updateGeometry() in the right place and that
there are no other places where it should be called? Having a test case clearly
demonstrating the connection between the docs quote and your last sentence of
the summary would be reassuring not
rkflx added a comment.
Looking at the Qt docs, we see:
> Call QWidget::updateGeometry() whenever the size hint, minimum size hint or
size policy changes. This will cause a layout recalculation.
To decide whether this is also meaningful for a squeezable label, a testcase
would be
rkflx updated this revision to Diff 17972.
rkflx added a comment.
Same wording for reimplementation warning as in
https://phabricator.kde.org/D7161.
Make indent, margin and lineWidth Q_PROPERTIES.
Add link to phabricator discussion as KF6 TODO (not sure if we can reach a
conclusion
rkflx added a comment.
Some thoughts, inconclusive though:
Another idea: Sometimes additional functionality for non-virtual functions is
provided by hooking into the changeEvent(). However, for our use case we won't
get such events from QLabel.
Things we might want to consider when
rkflx added inline comments.
INLINE COMMENTS
> dhaumann wrote in ksqueezedtextlabelautotest.cpp:217-234
> Looking at your other change requests: You may want to ignore the note about
> using setProperty(), since this will call QLabel::setIndent(), and not the
> one that you add in
rkflx marked 5 inline comments as done.
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
https://phabricator.kde.org/D7163
To: rkflx, #frameworks
Cc: dhaumann
rkflx updated this revision to Diff 17971.
rkflx added a comment.
Address comments:
- anonymous namespace
- QString() instead of QStringLiteral("")
- "pixels" for "amount"
- setProperty() instead of pointer to member function, remove switch and enum
REPOSITORY
R236 KWidgetsAddons
rkflx added a comment.
> I think this is already a very good patch. I just have some minor comments.
Thanks for looking at all these patches and taking the time for detailed
feedback. This is very helpful and really appreciated!
> you should consider applying for a KDE developer
rkflx marked an inline comment as done.
REPOSITORY
R236 KWidgetsAddons
BRANCH
arcpatch-D7162
REVISION DETAIL
https://phabricator.kde.org/D7162
To: rkflx, #frameworks, dhaumann
Cc: dhaumann
rkflx updated this revision to Diff 17970.
rkflx added a comment.
Improve wording to be as elegant as suggested by Dominik.
REPOSITORY
R236 KWidgetsAddons
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D7162?vs=17774=17970
BRANCH
arcpatch-D7162
REVISION DETAIL
rkflx updated this revision to Diff 17969.
rkflx added a comment.
> Mabye the "@note" about some methods being not virtual in the base class is
even worth to put into the main class documentation.
Good idea, let's move it to the class description (to be referenced from each
affected
12 matches
Mail list logo
