This revision was automatically updated to reflect the committed changes.
Closed by commit R236:ee67903f1369: KSqueezedTextLabel: Add several autotests
(authored by rkflx).
REPOSITORY
R236 KWidgetsAddons
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D7163?vs=19331&id=19379
REVISION
dhaumann accepted this revision.
This revision is now accepted and ready to land.
REPOSITORY
R236 KWidgetsAddons
BRANCH
arcpatch-D7163
REVISION DETAIL
https://phabricator.kde.org/D7163
To: rkflx, #frameworks, dhaumann
Cc: dhaumann
rkflx updated this revision to Diff 19331.
rkflx added a comment.
- wait for resize event after setting properties as discussed in
https://phabricator.kde.org/D7164#143540
- prevent leaks
REPOSITORY
R236 KWidgetsAddons
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D7163?vs=1797
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 KSqueezedText
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 acco
dhaumann added inline comments.
INLINE COMMENTS
> dhaumann wrote in ksqueezedtextlabelautotest.cpp:217-234
> margin, indent, and lineWidth are all Q_PROPERTYs, maybe you can use
> QObject::setProperty(const char *, QVariant) to set the values instead of
> using a function pointer? The function
dhaumann added a comment.
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 KSqueezedTextLabel (since setIndent() etc. are not virtual).
Maybe this needs further discuss
dhaumann added a comment.
I think this is already a very good patch. I just have some minor comments.
Could you have another look?
By the way: Given you have more patches that build on this one, you should
consider applying for a KDE developer account, if you don't have one yet, see:
ht
rkflx added a dependent revision: D7164: KSqueezedTextLabel: Respect indent,
margin and frame width.
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
https://phabricator.kde.org/D7163
To: rkflx, #frameworks
rkflx created this revision.
Restricted Application added a project: Frameworks.
REVISION SUMMARY
Tests most of the basic functionality and uncovers bugs for a few edge
cases. Testing some of the advanced features is skipped for now, there
might be more bugs lurking.
Checking the text e
12 matches
Mail list logo
