Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add a feature and fix a problem with dialogs and popups #23

Closed
wants to merge 5 commits into from
Closed

add a feature and fix a problem with dialogs and popups #23

wants to merge 5 commits into from

Conversation

freaksdd
Copy link
Contributor

I wrote two issues before but then i handle it by my self, cause i found a solution for this problems.

feature:

  • now the spacebar get a translation. Maybe there is not the right place, but for now i think there is okay.

Problem with Dialog and Popups:

Problem:
cuteKeyboard can not be use with Dialog and Popup. The Modal do the keyboard unusable cause of there excution.

Solution:
register the QQuickItem of InputPanel in the Backend and set the focusItem as parent of the InputPanel

@freaksdd
feat: use cutekeyboard in Dialog and Popup
72f46c8 
problem:
cuteKeyboard can not be use with Dialog and Popup. The Modal do the
keyboard unusable cause of there excution.

solution:
register the QQuickItem of InputPanel in the Backend and set the
focusItem as parent of the InputPanel
@freaksdd
feat: use cutekeyboard in Dialog and Popup (edit)
d2f0b72 
forgot to registrate the InputPanel in the backend
@freaksdd
feat: add translation for the space-bar
214f261 
add space-bar translation as 'spaceIdentifier'.
you can add the translation to the map in DeclarativeInputEngine.cpp, fallback 'space'
@freaksdd
fix: delete old item-connection
f6a9a10 
when there are manny items, there are a lot of connections in backend.
if the current item in action, the connection can be handle.
if a new item is focus, the 'old' connection can be overwritten.
@freaksdd
Add spaceIdentifier to QwertyLayout
926f8fe 
add missing spaceIdentifier to spaceLayout
AndreaRicchi
AndreaRicchi requested changes Aug 21, 2024
View reviewed changes
Copy link
Contributor

@AndreaRicchi AndreaRicchi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for the contribution. Just revert the Yocto recipe

yocto-recipes/cutekeyboard_git.bb Outdated
@@ -7,6 +7,6 @@ LIC_FILES_CHKSUM = "file://LICENSE;md5=2f6f4d4f7d76b223f08e17122d04010f"
DEPENDS += "qtbase qtdeclarative qtmultimedia qtquickcontrols qtsvg qtxmlpatterns"

SRCREV = "${AUTOREV}"
SRC_URI = "git://github.com/amarula/cutekeyboard.git;protocol=https;branch=main"
SRC_URI = "git://github.com/freaksdd/cutekeyboard.git;protocol=https;branch=main"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing the recipe URI cannot be merged. Revert this one

@freaksdd
Copy link
Contributor Author

I revert the change in Yocto recipe. Sorry for it. My knowledgement for versioncontrol are basic. So i don't understand with merges or request's from fork.

@AndreaRicchi AndreaRicchi self-requested a review August 22, 2024 07:16
@AndreaRicchi
Copy link
Contributor

@freaksdd I've tested the change in the example with this code:

    MessageDialog {
        id: dialog

        contentItem: RowLayout {
            TextField {
                placeholderText: ".."
            }

            Button {
                text: "ok"
                onClicked: close()
            }

        }

    }

but it doesn't work. Did you test with this kind of dialog?

@AndreaRicchi
Copy link
Contributor

Also, instead of having two commits

image

one that changes and one that reverts, remove both for a clearer history.

Thanks

@freaksdd
Copy link
Contributor Author

freaksdd commented Aug 29, 2024 via email

@AndreaRicchi
Copy link
Contributor

Hi, first, i' m from germany and my english are not so very well. I have only test it with Dialog{} and Popup{}. MessageDialog currently untestet. In therortic it have to be work, but i will look at the problem and tell you whats happen. Am Do., 29. Aug. 2024 um 08:32 Uhr schrieb Andrea Ricchi < @.>:
…
@freaksdd https://github.com/freaksdd I've tested the change in the example with this code: MessageDialog { id: dialog contentItem: RowLayout { TextField { placeholderText: ".." } Button { text: "ok" onClicked: close() } } } but it doesn't work. Did you test with this kind of dialog? — Reply to this email directly, view it on GitHub <#23 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADYU5FUQWBJI3LQXYUBY5W3ZT256XAVCNFSM6AAAAABMRTQFA2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMJWHAYTIOBUGY . You are receiving this because you were mentioned.Message ID: @.>

No problem, thanks for your contribution. I've also tried with a Dialog, this is the diff:

diff --git a/example/main.qml b/example/main.qml
index 65d572b..1250de1 100644
--- a/example/main.qml
+++ b/example/main.qml
@@ -1,6 +1,7 @@
 import QtQuick 2.12
 import QtQuick.Controls 2.12
 import QtQuick.CuteKeyboard 1.0
+import QtQuick.Dialogs 1.3
 import QtQuick.Layouts 1.12
 import QtQuick.Window 2.12
 
@@ -12,10 +13,34 @@ Window {
     height: 640
     title: qsTr("QtCuteKeyboard Example")
 
+    Dialog {
+        id: dialog
+
+        contentItem: RowLayout {
+            TextField {
+                placeholderText: ".."
+            }
+
+            Button {
+                text: "ok"
+                onClicked: close()
+            }
+
+        }
+
+    }
+
     ColumnLayout {
         spacing: 20
         anchors.centerIn: parent
 
+        Button {
+            width: 50
+            height: 50
+            text: "dialog"
+            onClicked: dialog.open()
+        }
+
         RowLayout {
             spacing: 10

Maybe we can try the same code 👍

@freaksdd
Copy link
Contributor Author

freaksdd commented Aug 29, 2024 via email

@freaksdd
Copy link
Contributor Author

freaksdd commented Aug 29, 2024 via email

@freaksdd
Copy link
Contributor Author

freaksdd commented Sep 10, 2024 via email

@freaksdd
Copy link
Contributor Author

freaksdd commented Sep 10, 2024 via email

AndreaRicchi
AndreaRicchi requested changes Sep 17, 2024
View reviewed changes
Copy link
Contributor

@AndreaRicchi AndreaRicchi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, still that is a good feature to be included.

Can you just improve the git history, so squash the 72f46c8 and d2f0b72 and also the 214f261 and 926f8fe .

You can use git rebase -i command.

I think merging a new feature with different commits for the same change is incorrect.

@freaksdd
Copy link
Contributor Author

freaksdd commented Sep 17, 2024 via email

@AndreaRicchi
Copy link
Contributor

Hi @freaksdd I've tried to fix the history but I don't want to merge it manually maybe losing your contribution score on the site, so I've squashed the commits with a new message:

commit e2cc2400855278c66f6bbf87f94fee71eab2e514 (HEAD -> freaksdd/main)
Author: freaksdd <[email protected]>
Date:   Thu Aug 15 07:23:44 2024 +0200

    Add translations for the SpaceBar
    
    Added SpaceBar translation with 'spaceIdentifier' property.
    
    The translation can be added to the map inside
    DeclarativeInputEngine.cpp, with 'space' as fallback.

commit 67b24dc275c0ccb560d08c80d3cafb723c0e4081
Author: freaksdd <[email protected]>
Date:   Thu Aug 15 07:35:09 2024 +0200

    fix: delete old item-connection
    
    when there are manny items, there are a lot of connections in backend.
    if the current item in action, the connection can be handle.
    if a new item is focus, the 'old' connection can be overwritten.

commit a11aafa4976d879df7fb9bcc30fa54799ffbded6
Author: freaksdd <[email protected]>
Date:   Wed Aug 14 14:25:58 2024 +0200

    Make Cutekeyboard compatible with Dialogs and Popup
    
    Because of the Modal option, the keyboard doesn't work with Dialogs
    and Popups.
    
    The solution is to register the QQuickItem of InputPanel
    in the Backend and set the focusItem as parent of the InputPanel.
    
    Note that the keyboard is still incompatible with QtQuick.Dialogs

I've used git rebase -i HEAD~5, moved the fix commit above and used the squash on the first 2 and latest 2.

I'll wait for your updates to merge in the main branch.

Regards

@freaksdd
Copy link
Contributor Author

freaksdd commented Sep 24, 2024 via email

@freaksdd
Copy link
Contributor Author

freaksdd commented Sep 24, 2024 via email

@AndreaRicchi
Copy link
Contributor

@freaksdd I've manually updated the commit messages and merged them.

Thanks

@freaksdd
Copy link
Contributor Author

freaksdd commented Sep 24, 2024 via email

dpurgin
dpurgin reviewed Sep 30, 2024
View reviewed changes
src/VirtualKeyboardInputContext.cpp
return;
}

// set the focusItem as parent of InputPanel
Copy link
Contributor

@dpurgin dpurgin Sep 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @AndreaRicchi

Please compare the changes in this file with the patch to qtvirtualkeyboard: https://codereview.qt-project.org/c/qt/qtvirtualkeyboard/+/292472/10 at src/virtualkeyboard/qvirtualkeyboardinputcontext_p.cpp. Apparently, the relevant changes were copied as-is into cutekeyboard. Could it be seen as a GPL violation?

@AndreaRicchi AndreaRicchi mentioned this pull request Oct 23, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dpurgin dpurgin dpurgin left review comments

@AndreaRicchi AndreaRicchi AndreaRicchi requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@freaksdd @AndreaRicchi @dpurgin