felixernst added a comment.
You all are too kind!
> Code looks fine.
First try \o/
I'll put the comments that aren't directly related into another revision.
I'll rename it to "--- expanding spacer ---" then. So I'll keep it lowercase
and in the same style as "--- seperator
ngraham added a comment.
+1 for adding both a fixed-width spacer as well as an expanding spacer.
REPOSITORY
R263 KXmlGui
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D22609
To: felixernst, dfaure
Cc: ngraham, #vdg, dfaure, kde-frameworks-devel, LeGast00n, sbergeron,
dfaure added a comment.
Ah yeah, about the name, I expected a fixed-width spacer until I saw the
video. And maybe someone wants to provide that as well... so "Expanding spacer"
would actually make sense for this feature.
Or maybe you even want to provide both right away, while at it...
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.
A video showing the feature, in the merge request! I'm blown away!
Code looks fine.
INLINE COMMENTS
> kxmlguibuilder.cpp:349
> +} else if (tagName == d->tagSpacer) {
> +if
ngraham added a comment.
Also, I would recommend doing most of the comment changes in a separate
patch. They are good and useful, but not related to adding this awesome new
spacer item.
REPOSITORY
R263 KXmlGui
REVISION DETAIL
https://phabricator.kde.org/D22609
To: felixernst
Cc:
ngraham added a comment.
+100!
How about naming it "Flexible space" or "Expanding spacer" to make it a bit
clearer?
REPOSITORY
R263 KXmlGui
REVISION DETAIL
https://phabricator.kde.org/D22609
To: felixernst
Cc: ngraham, #vdg, dfaure, kde-frameworks-devel, LeGast00n, sbergeron,
ngraham added subscribers: Frameworks, dfaure, VDG.
Herald removed a subscriber: Frameworks.
REPOSITORY
R263 KXmlGui
REVISION DETAIL
https://phabricator.kde.org/D22609
To: felixernst
Cc: #vdg, dfaure, kde-frameworks-devel, LeGast00n, sbergeron, michaelh,
ngraham, bruns
felixernst edited the summary of this revision.
REPOSITORY
R263 KXmlGui
REVISION DETAIL
https://phabricator.kde.org/D22609
To: felixernst
Cc: kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, bruns
felixernst updated this revision to Diff 62188.
felixernst added a comment.
Use insertWidget(before, spacer) instead of addWidget(spacer)
REPOSITORY
R263 KXmlGui
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D22609?vs=62185=62188
BRANCH
master
REVISION DETAIL
felixernst planned changes to this revision.
felixernst added a comment.
Need to fix wrong return type in KXMLGUIBuilder::createCustomElement
REPOSITORY
R263 KXmlGui
REVISION DETAIL
https://phabricator.kde.org/D22609
To: felixernst
Cc: kde-frameworks-devel, LeGast00n, sbergeron,
felixernst edited the test plan for this revision.
REPOSITORY
R263 KXmlGui
REVISION DETAIL
https://phabricator.kde.org/D22609
To: felixernst
Cc: kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, bruns
felixernst edited the summary of this revision.
REPOSITORY
R263 KXmlGui
REVISION DETAIL
https://phabricator.kde.org/D22609
To: felixernst
Cc: kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, bruns
felixernst edited the test plan for this revision.
REPOSITORY
R263 KXmlGui
REVISION DETAIL
https://phabricator.kde.org/D22609
To: felixernst
Cc: kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, bruns
felixernst created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
felixernst requested review of this revision.
REVISION SUMMARY
This commit adds spacers to the kxmlgui framework so all applications
using it will be able to use any amount
14 matches
Mail list logo