Port the dict applet to KF5 and QtQuick 2
ClosedPublic

Authored by dfaure on Oct 22 2017, 11:53 AM.

Details

Summary

My wife was missing this applet, after migrating to Plasma 5.
Therefore I had to learn how to write QML-based plasma applets :-)

Test Plan
  • Adding dict applet to desktop, works.
  • Looking up definitions, work.
  • Clicking on a link, works.
  • Hi-dpi support, works.
  • Configuration dialog to choose dictionary: done in a followup commit.

Diff Detail

Repository
R114 Plasma Addons
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
dfaure created this revision.Oct 22 2017, 11:53 AM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 22 2017, 11:53 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript

Cool, thanks!

Adding dict applet to desktop, works.

Have you added it to the panel?

applets/dict/package/contents/ui/main.qml
21

then use font metrics...http://doc.qt.io/qt-5/qml-qtquick-fontmetrics.html

or in this Plasmoid case use Units.gridUnit * 40

which is gonna be basically the same.

38

it's a bit rubbish, but:

zoom: Units.devicePixelRatio

should sort out DPI.
(not tested)

applets/dict/package/metadata.desktop
118–119

no it doesn't

applets/dict/plugin/dict_object.h
3

Plasma core provides a dataengine QML exposer.
https://api.kde.org/frameworks/plasma-framework/html/classPlasma_1_1DataSource.html

Is there a reason we made a specialised QML wrapper?
(not that I object to this)

broulik added inline comments.
applets/dict/package/contents/ui/main.qml
5

Unused?

23

onAccepted instead of Keys.onReturnPressed

34

Does this need a Layout.fillWidth: true and Layout.fillHeight: true? The TextField probably also needs Layout.fillWidth?

dfaure marked an inline comment as done.Oct 27 2017, 8:59 PM
dfaure added inline comments.
applets/dict/package/contents/ui/main.qml
5

Ah, yes. That was for "formFactor" as in https://techbase.kde.org/Development/Tutorials/Plasma5/QML2/GettingStarted#Minimum_size
(which I had to fix so the syntax would parse).

But I'm not sure if I need to handle form factors in this applet, and if so, how...

21

Oh, cool, FontMetrics didn't exist in my first QML attempts.

But yeah if plasmoids should fit a grid, I'll use the grid then.

34

Layout.fillHeight: true led to a 1-pixel-high webview, not good.

Layout.fillWidth: true is good, it means I don't duplicate the width anymore.

OK, and doing that in the TextField makes the applet resize to the user's wishes, I see. Thanks.

38

zoomFactor: units.devicePixelRatio
helped indeed, thanks.

applets/dict/package/metadata.desktop
118–119

Indeed. One day it should, though, I'd like the user to be able to choose which dict to use. Do I have to write a config dialog in QML, though? If so, any example I can use for that?
Or can it use widgets?

applets/dict/plugin/dict_object.h
3

Ah, I didn't know.
But I'd rather have as much logic as possible in C++, rather than in JS.
The whole thing about listing dicts, and allowing the user to choose one (or more?), storing that in KConfig, and then using that for querying... I'm not sure but I think using DataSource directly wouldn't make this easy, I would still need a QObject at least for the KConfig part, and it would mean interfacing that with DataSource in JS code...

dfaure added inline comments.Oct 27 2017, 9:12 PM
applets/dict/package/contents/ui/main.qml
34

Hmm, but the user should be able to resize the applet vertically too... I get very confusing behaviour even with
`Layout.preferredHeight: 400
Layout.fillHeight: true`

It resizes down to very little, jumps around in the desktop, etc.
I need to dig further into all this.

dfaure updated this revision to Diff 21473.Oct 28 2017, 8:37 AM

Add support for clicking on links; improve layout a bit

Have you added it to the panel?

What should happen when I do? Surely a webview won't fit there...

dfaure edited the test plan for this revision. (Show Details)Nov 23 2017, 7:35 PM
dfaure updated this revision to Diff 22852.Nov 23 2017, 7:36 PM
dfaure edited the test plan for this revision. (Show Details)

clean up TODO

dfaure updated this revision to Diff 22855.Nov 23 2017, 8:26 PM

Remove old code, not used anymore.

What should happen when I do? Surely a webview won't fit there...

We should show an icon (like a dictionary or something)
which will show the popup with the relevant plasmoid.

In theory this should all happen automatically if the minimum size hints of your full main.qml are too massive for a panel and we get a "compact representation" instead.

anthonyfieroni added inline comments.
applets/dict/plugin/dict_object.cpp
43

Why you prefer invoke rather than a signal ?

What should happen when I do? Surely a webview won't fit there...

We should show an icon (like a dictionary or something)
which will show the popup with the relevant plasmoid.

In theory this should all happen automatically if the minimum size hints of your full main.qml are too massive for a panel and we get a "compact representation" instead.

Hey that actually works out of the box, it's magic :-)

applets/dict/plugin/dict_object.cpp
43

Heh, I just didn't think of that, when I turned the direct call into invokeMethod to fix the threading issue. Indeed a signal is much simpler, removes the need for m_dict altogether.
Thanks for the suggestion.

dfaure updated this revision to Diff 22860.Nov 23 2017, 10:08 PM

Use a signal instead of invokeMethod

dfaure edited the test plan for this revision. (Show Details)Dec 2 2017, 10:33 AM
davidedmundson accepted this revision.Dec 2 2017, 10:50 AM
This revision is now accepted and ready to land.Dec 2 2017, 10:50 AM
This revision was automatically updated to reflect the committed changes.