D22609: Add spacers as a customization option for toolbars
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 ---" if nobody has a better idea. > fixed-width spacer I don't really understand their benefit yet because I can't imagine a scenario where I would want one that wouldn't better be solved with an expanding one. So to me it seems like it is a widget we don't want to have so users don't pick the spacer that is worse in 95 % of cases out of lack of knowledge. I can be convinced to add a fixed-width one though if I see an example where we would want them. We would have to decide what size a fixed-width spacer has. REPOSITORY R263 KXmlGui BRANCH master REVISION DETAIL https://phabricator.kde.org/D22609 To: felixernst, dfaure Cc: ngraham, #vdg, dfaure, kde-frameworks-devel, LeGast00n, sbergeron, michaelh, bruns
D22609: Add spacers as a customization option for toolbars
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, michaelh, bruns
D22609: Add spacers as a customization option for toolbars
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... REPOSITORY R263 KXmlGui BRANCH master REVISION DETAIL https://phabricator.kde.org/D22609 To: felixernst, dfaure Cc: ngraham, #vdg, dfaure, kde-frameworks-devel, LeGast00n, sbergeron, michaelh, bruns
D22609: Add spacers as a customization option for toolbars
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 (KToolBar *bar = qobject_cast(parent)) {
> +// Create the simple spacer widget
(pre-existing, in the separator code above) QToolBar would be sufficient,
technically, in this code.
REPOSITORY
R263 KXmlGui
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D22609
To: felixernst, dfaure
Cc: ngraham, #vdg, dfaure, kde-frameworks-devel, LeGast00n, sbergeron,
michaelh, bruns
D22609: Add spacers as a customization option for toolbars
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, #vdg, dfaure, kde-frameworks-devel, LeGast00n, sbergeron, michaelh, bruns
D22609: Add spacers as a customization option for toolbars
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, michaelh, bruns
D22609: Add spacers as a customization option for toolbars
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
D22609: Add spacers as a customization option for toolbars
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
D22609: Add spacers as a customization option for toolbars
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 https://phabricator.kde.org/D22609 AFFECTED FILES src/kedittoolbar.cpp src/kedittoolbar.h src/kxmlgui.xsd src/kxmlguibuilder.cpp To: felixernst Cc: kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, bruns
D22609: Add spacers as a customization option for toolbars
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, michaelh, ngraham, bruns
D22609: Add spacers as a customization option for toolbars
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
D22609: Add spacers as a customization option for toolbars
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
D22609: Add spacers as a customization option for toolbars
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
D22609: Add spacers as a customization option for toolbars
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 of spacers in their toolbar(s). KEditToolbar gets the --- spacer --- entry by default. This entry is modified to allow any amount of spacers to be put into the toolbars (just like separators). The xml scheme is changed to allow "" nodes (just like separators). KXmlGuiBuilder then builds the simple spacer by itself. REPOSITORY R263 KXmlGui BRANCH master REVISION DETAIL https://phabricator.kde.org/D22609 AFFECTED FILES src/kedittoolbar.cpp src/kedittoolbar.h src/kxmlgui.xsd src/kxmlguibuilder.cpp To: felixernst Cc: kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, bruns
