[WIP] Add Date/Time dialog
Needs ReviewPublic

Authored by nicolasfella on Apr 28 2020, 7:42 PM.

Details

Summary

Import the date/time picker used in KTrip. It wraps the existing date/time picker components into a dialog. On Android it calls the native date/time picker instead.

The Android implementation is slightly messy since we apparently can't do JNI things directly from a QML plugin because it gets loaded in the wrong thread. As a workaround we have a linkable library on Android that forces JNI initialization in the right thread.

TODO: docs

Test Plan

Ported KTrip to it

Diff Detail

Repository
R1047 Kirigami Addons
Branch
work
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 26056
Build 26074: arc lint + arc unit
nicolasfella created this revision.Apr 28 2020, 7:42 PM
Restricted Application added a project: Plasma. · View Herald TranscriptApr 28 2020, 7:42 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
nicolasfella requested review of this revision.Apr 28 2020, 7:42 PM
nicolasfella edited the summary of this revision. (Show Details)Apr 28 2020, 7:44 PM
broulik added inline comments.Apr 29 2020, 6:56 AM
CMakeLists.txt
69

Why this only on Android? or is that used for that dummy library?

cmake/modules/FindGradle.cmake
2

Feels like this should go into ECM?

src/dateandtime/DateDialog.qml
12

org.kde.kirigamiaddons.private.dateandtime to emphasis it's an impl detail

15

theDate is a bit awkward since that's what you'd have to use when you use the signal

src/dateandtime/KF5KirigamiDateAndTimeAndroid-android-dependencies.xml
5

KF5KirigamiAddonsDateAndTime?

src/dateandtime/TimeDialog.qml
15

Generally conversions between C++ timedate and JavaScript Date is bad. There's no way to represent just a time with no date and timezone associated with it in JavaScript.
While it's ugly, I'd suggest we return a bunch of int.
Also, I think we should have the selected time as properties and an accepted/rejected signal or similar.
(Same probably goes for the date picker)

35

Use QtObject for this then

src/dateandtime/lib/androidutils.cpp
12

Unused

54

dateMethods

61

Coding style

66

Better name please

89

There's QDateTime::currentMSecsSinceEpoch()

src/dateandtime/lib/androidutils.h
10

Just forward-declare?

26

Generally we want singletons to return references these days to communicate ownership properly

29

const QDate &

src/dateandtime/lib/plugin.cpp
58

You need to set

QQmlEngine::setObjectOwnership(..., QQmlEngine::CppOwnership);

A QObject singleton type instance returned from a singleton type provider is owned by the QML engine unless the object has explicit QQmlEngine::CppOwnership flag set.

There might be ways around the native function registration issue from the QML thread, e.g. by using the alternative approach of exported (mangled) symbols instead: https://docs.oracle.com/javase/1.5.0/docs/guide/jni/spec/design.html -> "Loading and Linking Native Methods".

There might be ways around the native function registration issue from the QML thread, e.g. by using the alternative approach of exported (mangled) symbols instead: https://docs.oracle.com/javase/1.5.0/docs/guide/jni/spec/design.html -> "Loading and Linking Native Methods".

I tried that, but it didn't seem to work.

nicolasfella added inline comments.May 18 2020, 6:40 PM
CMakeLists.txt
69

It's for the dummy library only

cmake/modules/FindGradle.cmake
2

Yep

src/dateandtime/DateDialog.qml
12

I'd rather do that in a separate patch since it should also be done for other classes

src/dateandtime/TimeDialog.qml
15

In this particular case the timezone is not relevant though

nicolasfella marked 10 inline comments as done.
  • fix
  • Address some comments
nicolasfella marked an inline comment as done.May 18 2020, 7:39 PM
nicolasfella added inline comments.
src/dateandtime/TimeDialog.qml
15

If we do this should the date thing also use ints for symmentry?

nicolasfella added inline comments.May 18 2020, 8:50 PM
src/dateandtime/lib/androidutils.h
29

Actually clazy disagrees, QDate and QTime should be passed by value