Does Spectacle belong to a certain review group?

Yup!

This looks great! We're currently in the process of re-doing the buttons on the bottom (T7841), so let's get that done fist and then we can investigate this.

In D10295#200663, @ngraham wrote:

This looks great! We're currently in the process of re-doing the buttons on the bottom (T7841), so let's get that done fist and then we can investigate this.

The important bits are now done in D10371 (either wait until it landed, or "Depend on" it).

Do KMoreTools allow to add it like this?:

• Print
• ---------
• Record screen with Peek
• Record screen with SimpleScreenRecorder
• Configure Tools...

We've got our tools menu now but it's looking a little bit sad with only a Print item. This would be a great time to rebase on master and put the screen recording items in there!

Also, I know some volks like Vokoscreen (ah-ha-ha); is there any way we can support that too, or is that up to KNewStuff or the Vokoscreen developer?

In D10295#202617, @rkflx wrote:

Do KMoreTools allow to add it like this?:

• Print
• ---------
• Record screen with Peek
• Record screen with SimpleScreenRecorder
• Configure Tools...

I think it is possible with the current KMoreTools API but it the code not be nice. Better would be extend the the API for this use case.

One thing should be considered with the proposed menu layout: if the user chooses "Configure Tools..." this would only configure the "Record screen" tools and not also the Print tool. If one wanted to let the user configure the whole menu then everything should be moved to KMoreTools.

This alternative would work out of the box but comes with one more nesting in the menu:

• Print
• Record screen video
  • Peek
  • SimpleScreenRecorder
  • Vokoscreen
  • Configure Tools...

In D10295#202788, @ngraham wrote:

Also, I know some volks like Vokoscreen (ah-ha-ha); is there any way we can support that too, or is that up to KNewStuff or the Vokoscreen developer?

The "screenrecorder" grouping with Peek and SimpleScreenrecorder was added to KMoreTools with this patch (among other things): https://phabricator.kde.org/D7130. Vokoscreen can be added the same way. So it is up to KNewStuff/KMoreTools.

Instead of "Record Screen Video", how about "Make Screen Recording" or just "Record screen"?

In D10295#203073, @ngraham wrote:

Instead of "Record Screen Video", how about "Make Screen Recording" or just "Record screen"?

I thought that just "Record screen" could be confused with to take (or record) a still image of the screen. But I am no native speaker. So, you decide. :-)

"Record" in English always has a time component when talking about some form of media. You would record a video, but you wouldn't record an image.

FYI You can update the screenshot in the summary section if you'd like.

Here's how it looks for me:

I'd really love to omit one level of extra nesting.

In D10295#203041, @gregormi wrote:

In D10295#202617, @rkflx wrote:

Do KMoreTools allow to add it like this?:

• Print
• ---------
• Record screen with Peek
• Record screen with SimpleScreenRecorder
• Configure Tools...

I think it is possible with the current KMoreTools API but it the code not be nice. Better would be extend the the API for this use case.

Dependency Freeze for KDE Applications 18.04 is on March 15, meaning you'd have to get that done for KDE Frameworks 5.44 on March 3. Do you think you could do that? If not, I guess the current solution would be fine. What would "not nice code" entail?

if the user chooses "Configure Tools..." this would only configure the "Record screen" tools and not also the Print tool.

I know, that's why I proposed to have the horizontal separator. Of course then there should be no separator between the screen recording tools and the configure button ;)

Do users actually have both tools installed? If not, it's showing "More" anyway instead of "Configure".

Also: "Configure" clashes with the regular Configure button. Could you rename it to something like Configure external tools?

In D10295#203073, @ngraham wrote:

Instead of "Record Screen Video", how about "Make Screen Recording" or just "Record screen"?

Yeah, that's what I stumbled upon too. Wikipedia has it as "Screencasting", but "Record screen" is probably fine.

nesting

You could also think about QMenu::addSection.

In D10295#203095, @rkflx wrote:

Here's how it looks for me:

I'd really love to omit one level of extra nesting.

Yes, I know KMoreTools comes with too much nesting in menus :-(. Especially when there are few items it not really looks good.

I think it is possible with the current KMoreTools API but it the code not be nice. Better would be extend the the API for this use case.

Dependency Freeze for KDE Applications 18.04 is on March 15, meaning you'd have to get that done for KDE Frameworks 5.44 on March 3. Do you think you could do that? If not, I guess the current solution would be fine. What would "not nice code" entail?

Meeting the deadline: no. I would have to familiarize again with the current API and come up with a good ideas.

"not nice code": I designed KMoreTools to be very flexible but sadly this comes with a complicated API. Up to now, this flexibility was not used and I think it would be better to deprecate some of the API and add better ones. E.g. currently, the relevant line in Spectacle is:

this->mScreenrecorderToolsMenuFactory->fillMenuFromGroupingNames(mScreenRecorderToolsMenu, { QStringLiteral("screenrecorder") });

This could be replaced with something like this:

this->mScreenrecorderToolsMenuFactory->fillMenuWithPrefixedItemsFromGroupingNames(mScreenRecorderToolsMenu, i18n("Record screen with "), { QStringLiteral("screenrecorder") });

if the user chooses "Configure Tools..." this would only configure the "Record screen" tools and not also the Print tool.

I know, that's why I proposed to have the horizontal separator. Of course then there should be no separator between the screen recording tools and the configure button ;)

Sure :)

Do users actually have both tools installed? If not, it's showing "More" anyway instead of "Configure".

Yes, the idea was that "Configure" should be as unobstrusive as possible. I even thought about hiding it completely and only show it when the user presses a key like "Shift". Some discussions are needed to make this better :)

Also: "Configure" clashes with the regular Configure button. Could you rename it to something like Configure external tools?

From KMoreTools' view: the question here would be if the name "Configure" should be configurable or if it should be globally renamed.

In D10295#203073, @ngraham wrote:

Instead of "Record Screen Video", how about "Make Screen Recording" or just "Record screen"?

Yeah, that's what I stumbled upon too. Wikipedia has it as "Screencasting", but "Record screen" is probably fine.

This can be changed anytime.

I underestimated the current API :). Now it looks not really perfect but much better:

Cool! I'm still not 100% convinced what variant to ship for 18.04, though.

@ngraham How do you like it now?

With no tools available at all it looks a bit weird, but I guess that's also something that should be improved in KMoreTools eventually (along with prefixing, separate calls for app entries vs. "management" entries, more flexible naming, less nesting, integration with Discover etc.).

In D10295#203115, @gregormi wrote:

From KMoreTools' view: the question here would be if the name "Configure" should be configurable or if it should be globally renamed.

Idea: Edit Entries – This should reduce the chances of clashing greatly. Apart from that, this depends on where it is used currently and where you want to use it in the future (I cannot answer that ;).

The centered "Record Screen" title in the menu is somehow bothering me. Doesn't feel quite right. Might be nicer if it were left-aligned, had a colon on the end of it, and it was grayed out. That way it would look more like a header introducing a section and less like a weird menu item.

In D10295#203139, @ngraham wrote:

The centered "Record Screen" title in the menu is somehow bothering me. Doesn't feel quite right. Might be nicer if it were left-aligned, had a colon on the end of it, and it was grayed out. That way it would look more like a header introducing a section and less like a weird menu item.

I fear that's how Breeze works…

Let's step back a bit: What's the common case? None of the apps is installed, I guess (correct me if I'm wrong). Then it looks bad, so I'd say for now we use the nested menu, and improve things in KMoreTools before making Spectacle better.

Thoughts?

In D10295#203141, @rkflx wrote:

In D10295#203139, @ngraham wrote:

The centered "Record Screen" title in the menu is somehow bothering me. Doesn't feel quite right. Might be nicer if it were left-aligned, had a colon on the end of it, and it was grayed out. That way it would look more like a header introducing a section and less like a weird menu item.

I fear that's how Breeze works…

OK, you're right. I notice the same pattern used in Kate. Let;s not tinker with it; consistency is more important.

Let's step back a bit: What's the common case? None of the apps is installed, I guess (correct me if I'm wrong).

Yes, I agree that we need to optimize for this case. Does KNS open Discover with the app in question if you opt to install it?

In D10295#203141, @rkflx wrote:

In D10295#203139, @ngraham wrote:

The centered "Record Screen" title in the menu is somehow bothering me. Doesn't feel quite right. Might be nicer if it were left-aligned, had a colon on the end of it, and it was grayed out. That way it would look more like a header introducing a section and less like a weird menu item.

Me too.

I fear that's how Breeze works…

I also thought that.

Let's step back a bit: What's the common case? None of the apps is installed, I guess (correct me if I'm wrong). Then it looks bad, so I'd say for now we use the nested menu, and improve things in KMoreTools before making Spectacle better.

No apps installed:

Thoughts?

In D10295#203142, @ngraham wrote:

Yes, I agree that we need to optimize for this case. Does KNS open Discover with the app in question if you opt to install it?

For me it will open the browser, see D10295#203095. Do you see any appstream URL in D10390?

In D10295#203144, @gregormi wrote:

In D10295#203141, @rkflx wrote:

In D10295#203139, @ngraham wrote:

The centered "Record Screen" title in the menu is somehow bothering me. Doesn't feel quite right. Might be nicer if it were left-aligned, had a colon on the end of it, and it was grayed out. That way it would look more like a header introducing a section and less like a weird menu item.

Me too.

I fear that's how Breeze works…

I also thought that.

Let's step back a bit: What's the common case? None of the apps is installed, I guess (correct me if I'm wrong). Then it looks bad, so I'd say for now we use the nested menu, and improve things in KMoreTools before making Spectacle better.

No apps installed:

Thoughts?

The nesting looks fine to me. There are two visual papercuts I'd like to see addressed, and then I'd +1 that for the "no apps installed" case:

• Double lines above "More"
• The word "More" is not very descriptive. For now, since the only thing it lets you do is download screen recording tools, how about changing that to "Record screen", which will give a preview of the available functionality?

In D10295#203157, @rkflx wrote:

In D10295#203142, @ngraham wrote:

Yes, I agree that we need to optimize for this case. Does KNS open Discover with the app in question if you opt to install it?

For me it will open the browser, see D10295#203095. Do you see any appstream URL in D10390?

That's pretty useless then; Linux users don't install apps from random websites! We need to fix this in KNewStuff!

In D10295#203142, @ngraham wrote:

Let's step back a bit: What's the common case? None of the apps is installed, I guess (correct me if I'm wrong).

Yes, I agree that we need to optimize for this case. Does KNS open Discover with the app in question if you opt to install it?

I also see this as a problem. However, I am not sure if I currently have the time to work on KMoreTools to help to make it better. But this would mean we cannot get the Record Screen feature shipped which I find not really satisfactory.

In D10295#203160, @ngraham wrote:

In D10295#203157, @rkflx wrote:

In D10295#203142, @ngraham wrote:

Yes, I agree that we need to optimize for this case. Does KNS open Discover with the app in question if you opt to install it?

For me it will open the browser, see D10295#203095. Do you see any appstream URL in D10390?

That's pretty useless then; Linux users don't install apps from random websites! We need to fix this in KNewStuff!

Definitely. Discover integration would be the killer feature.

In D10295#203164, @gregormi wrote:

In D10295#203160, @ngraham wrote:

In D10295#203157, @rkflx wrote:

In D10295#203142, @ngraham wrote:

Yes, I agree that we need to optimize for this case. Does KNS open Discover with the app in question if you opt to install it?

For me it will open the browser, see D10295#203095. Do you see any appstream URL in D10390?

There is none (appstream URL), because at that time I had no idea of how to do it in a good way.

That's pretty useless then; Linux users don't install apps from random websites! We need to fix this in KNewStuff!

Definitely. Discover integration would be the killer feature.

Maybe, the website should still be present if there is no appstream info yet.

@ngraham It's either addSection with the bugs, or nesting. All other changes (e.g. to get it to look like my original proposal in D10295#202617) are not possible inside Spectacle safe for ugly hacks.

My vote for 18.04 is for nesting.

I won't argue, nesting is fine.

So back to nesting?

I'm not a huge fan, but that's the more standard way of doing it (See Dolphin's Free Space widget in the bottom right corner), so if we improve KNS to have a better UI, spectacle should be able to just inherit that.

In D10295#203171, @gregormi wrote:

So back to nesting?

I guess so. At least we tried, and identified what needs to be improved.

@gregormi It's fine if you are short on time to work on this, but it would be a shame if the ideas got lost in a random Phabricator comment. Could you open tasks on some Phab workboard to at least track this?

In D10295#203176, @rkflx wrote:

In D10295#203171, @gregormi wrote:

So back to nesting?

I guess so. At least we tried, and identified what needs to be improved.

@gregormi It's fine if you are short on time to work on this, but it would be a shame if the ideas got lost in a random Phabricator comment. Could you open tasks on some Phab workboard to at least track this?

I did not work with Phabricator workboards before. Is there a specific one where I could add these ideas?

In D10295#203187, @gregormi wrote:

I did not work with Phabricator workboards before. Is there a specific one where I could add these ideas?

Sorry, I should've done the work of looking for a board. As KNewStuff has its own project on Phab, just use the one over there: https://phabricator.kde.org/project/view/127/.

Thanks ;)

(Review of Diff 26773.)

Forgot something: It would be great if you could add a short mention of your new feature to doc/index.docbook (grep for <guibutton>Tools</guibutton>).

@gregormi D10301 just landed, you might want to watch out for docbook merge conflicts.

@gregormi Just so you know, this is the last patch that needs to merge before I can publish a Usability & Productivity blog post about all the awesome Spectacle changes recently!

@rkflx: May I ask what is your toolchain when dealing with docbook? I found out this:

cd kde/src/kde/kdegraphics/spectacle/doc/ ; dblatex index.docbook ; xdg-open index.docbook.pdf

which gives a rough but usable preview.

When I click the Help button from the dev version of Spectacle, then KHelpCenter opens but it shows the German version of the Spectacle Handbook. But the application language of KHelpCenter is English.

In D10295#203190, @rkflx wrote:

[... ] As KNewStuff has its own project on Phab, just use the one over there: https://phabricator.kde.org/project/view/127/.

Thanks ;)

I created this task: https://phabricator.kde.org/T7971

In D10295#204649, @rkflx wrote:

@gregormi D10301 just landed, you might want to watch out for docbook merge conflicts.

Mhm, seems like we lost a bunch of changes. Not sure what's gone wrong, either bad editing of merge conflict or rebase on stale master instead of fresh origin/master?

Anyway, I'll open a Diff to fix this tonight. Then we can also figure out the best order of the tools menu's items, and move everything to // the tools menu.

In D10295#206624, @gregormi wrote:

@rkflx: May I ask what is your toolchain when dealing with docbook? I found out this:

cd kde/src/kde/kdegraphics/spectacle/doc/ ; dblatex index.docbook ; xdg-open index.docbook.pdf

which gives a rough but usable preview.

Well, I make install (all or just from doc) to a custom prefix with -DCMAKE_INSTALL_PREFIX=~/path, and then either press F1 in Spectacle or issue khelpcenter help:/spectacle/additional-functionality.html#id1580 (found via Copy Link Address). You need to set some paths for this to work (quite complicated, I know).

The PDF workflow is not bad, but unfortunately it misses some markup (e.g. keyboard shortcuts, menu items).

Related (directly dumped from my notes, because I stumbled upon this topic just like you a couple of weeks ago):

• checkXML5 index.docbook
• meinproc5 --stylesheet /usr/share/kf5/kdoctools/customization/kde-chunk-online.xsl index.docbook
• https://l10n.kde.org/docs/tools.php

When I click the Help button from the dev version of Spectacle, then KHelpCenter opens but it shows the German version of the Spectacle Handbook. But the application language of KHelpCenter is English.

I just set my Plasma session to non-English and installed spectacle-lang to get translations. For make uninstall, I get the translated version, for make install I get the updated English version. AFAIK KHelpCenter relies on the help:/ kioslave, which looks in the standard paths like it should. Either move the relevant files there (/usr/share/... or ~/.local/...), or add your custom install prefix to the standard paths like in the link I mentioned.

Any volunteers to update some wiki pages? T7116#128201 suggests it's still a bit hard to find, or rather we need a developer-oriented cookbook for quick verification of docbook changes (as well as a better version of the env paths link)… @ngraham?

In D10295#206794, @gregormi wrote:

I created this task: https://phabricator.kde.org/T7971

Thanks ;)

(Note you could simply omit https://phabricator.kde.org/, and then Phabricator will do the link for you as well as add a "mention" to the object you linked to, which makes it easier to find related things…)

In D10295#206872, @rkflx wrote:

In D10295#204649, @rkflx wrote:

@gregormi D10301 just landed, you might want to watch out for docbook merge conflicts.

Mhm, seems like we lost a bunch of changes. Not sure what's gone wrong, either bad editing of merge conflict or rebase on stale master instead of fresh origin/master?

You mean commits you made are lost because I did an $ arc land? Or are we talking about changes within this Differential revision?

Anyway, I'll open a Diff to fix this tonight. Then we can also figure out the best order of the tools menu's items, and move everything to // the tools menu.

ok.

I identified some of the lost changes in index.docbook and added a new patch: https://phabricator.kde.org/D10542