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, #framew
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 o
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 goo
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 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
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&id=17970
BRANCH
arcpatch-D7162
REVISION DETAIL
https://phab
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 func
rkflx edited the summary of this revision.
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
https://phabricator.kde.org/D7162
To: rkflx, #frameworks
rkflx added a dependent revision: D7162: KSqueezedTextLabel: Add isSqueezed()
for convenience.
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
https://phabricator.kde.org/D7161
To: rkflx, #frameworks
rkflx edited the summary of this revision.
rkflx added a dependency: D7161: KSqueezedTextLabel: Small improvements to API
docs.
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
https://phabricator.kde.org/D7162
To: rkflx, #frameworks
rkflx added a dependent revision: D6696: Elide cut off text in sidebar header,
remove restricted max width.
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
https://phabricator.kde.org/D7164
To: rkflx, #frameworks
rkflx created this revision.
Restricted Application added a project: Frameworks.
REVISION SUMMARY
Text could display cut off when setting indent, margin and/or frame of
the label. On top of that, even when following the size hint to display
the complete text, text could be elided.
This
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
rkflx added a dependent revision: D7163: KSqueezedTextLabel: Add several
autotests.
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
https://phabricator.kde.org/D7162
To: rkflx, #frameworks
rkflx created this revision.
Restricted Application added a project: Frameworks.
REVISION SUMMARY
More readable than having to check fullText!=text() again and again.
Will be used in upcoming autotests, but should be useful in general.
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
ht
rkflx created this revision.
Restricted Application added a project: Frameworks.
REVISION SUMMARY
Expand description, add missing documentation.
"Above" in source code does not imply "above" in generated API docs,
better use a reference.
TEST PLAN
Output of kapidox looks better.
REPOS
22 matches
Mail list logo
