Bug 463247 - Unable to create new KDE CONNECT SMS messages with "+New" - button greyed out
Summary: Unable to create new KDE CONNECT SMS messages with "+New" - button greyed out
Status: REOPENED
Alias: None
Product: kdeconnect
Classification: Applications
Component: messaging-application (other bugs)
Version First Reported In: 22.12.0
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Simeet Nayan
URL:
Keywords: junior-jobs
Depends on:
Blocks:
 
Reported: 2022-12-19 21:26 UTC by Rigoberto Leyva Salmeron
Modified: 2023-08-19 11:48 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Rigoberto Leyva Salmeron 2022-12-19 21:26:59 UTC
SUMMARY
Not able to create new messages directly on KDE CONNECT SMS application, they "+New" button is greyed out. 


STEPS TO REPRODUCE
1. Open KDE Connect > SMS Messages
2. +New button appears greyed out


OBSERVED RESULT
When Opening KDE Connect > SMS Messages +New button appears greyed out

EXPECTED RESULT
When Opening KDE Connect > SMS Messages +New button is available and can create new SMS Message

SOFTWARE/OS VERSIONS
Windows: 
macOS: 
Linux/KDE Plasma:  KDE NEON 
(available in About System)
KDE Plasma Version: 5.26.4
KDE Frameworks Version: 5.101.0
Qt Version: 5.15.7

ADDITIONAL INFORMATION
Comment 1 Simon Redman 2022-12-27 03:47:54 UTC
Thanks for reporting this. I see the same behavior. I remember there was a refactor of the UI awhile ago, I expect the `enabled` property of the New button got disconnected somehow as part of that.
Comment 2 Simon Redman 2022-12-27 03:58:30 UTC
Marking this as a junior job. It should be a simple fix, poke into the QML and find why the logic for the `enabled` state of the new button doesn't work. If I were guessing, it's probably due to a mis-named or re-named variable.
Comment 3 Simeet Nayan 2023-02-06 12:17:13 UTC
Hi. I worked through the code and found that new button would be enabled only when you put in a right address in the text field. Enabling button without checking the address does not make sense logically. Doing so will make it look like a bug.
Comment 4 Rigoberto Leyva Salmeron 2023-02-06 18:14:12 UTC
`+New`  does become active after typing into the text box on top left of app; however, nothing happens after pressing the button.

Steps to reproduce:
1. New phone number typed inside the text box on top left of app -  the `+New` button  becomes active
2. Press `+New` button
3. Nothing happens

Expected results:
1. New phone number typed inside the text box on top left of app -  the `+New` button  becomes active
2. Press `+New` button
3. New window opens on the right panel of the app for a new text message to start

Note: Clicking the phone number typed on the left lower part of the app does open a new window on the right panel of the app.

Should a new bug be filed?
Comment 5 Simeet Nayan 2023-02-06 19:04:15 UTC
(In reply to Rigoberto Leyva Salmeron from comment #4)
> `+New`  does become active after typing into the text box on top left of
> app; however, nothing happens after pressing the button.
> 
> Steps to reproduce:
> 1. New phone number typed inside the text box on top left of app -  the
> `+New` button  becomes active
> 2. Press `+New` button
> 3. Nothing happens
> 
> Expected results:
> 1. New phone number typed inside the text box on top left of app -  the
> `+New` button  becomes active
> 2. Press `+New` button
> 3. New window opens on the right panel of the app for a new text message to
> start
> 
> Note: Clicking the phone number typed on the left lower part of the app does
> open a new window on the right panel of the app.
> 
> Should a new bug be filed?

Actually the new button is for "starting a conversation" as the tooltip says on hovering over it. On clicking "+ New" button a new conversation is already started. Clicking on the conversation brings you to a window where you can send messages. I think you are misunderstanding starting a conversation with a new window for sending messages.
Comment 6 Rigoberto Leyva Salmeron 2023-02-06 19:25:44 UTC
Do not understand the functionality of `+New`

1. Existing conversation, one clicks on the name of the conversation on the lower left panel.
2. New conversation, type the new phone number in the text box and click on the phone number that appears on the lower left panel.

How is the `+New` button used? - one would expect that +New starts a new conversation similar to `New Chat` on Android
Comment 7 Simeet Nayan 2023-02-06 20:07:18 UTC
(In reply to Rigoberto Leyva Salmeron from comment #6)
> Do not understand the functionality of `+New`
> 
> 1. Existing conversation, one clicks on the name of the conversation on the
> lower left panel.
> 2. New conversation, type the new phone number in the text box and click on
> the phone number that appears on the lower left panel.
> 
> How is the `+New` button used? - one would expect that +New starts a new
> conversation similar to `New Chat` on Android

Just enter any number in the text box. Click on "+ New". Now a new conversation is started(The number will now be highlighted). Click on the conversation and then a new window will come up where you can send message to the number.
Comment 8 Rigoberto Leyva Salmeron 2023-02-06 20:45:21 UTC
Ok,  your steps works for phone numbers not in my contacts only

For phone numbers I have in my contacts,  after number is typed in text box,  "The number is highlighted" > then I have to Click on the conversation so the new window comes up on the right side of the app to type the conversation - no need to click on `+New` button. 


BTW: Not sure why user has to make an extra click when starting a new conversation. If it is a new phone number for which there is no history of text messages, then clicking `+New` should get the conversation started - instead of 1. click `+New`  and 2. click the  "The number which is highlighted"
Comment 9 Phani Pavan K 2023-08-02 04:53:26 UTC
My Proposal:

P1. The "+New" button appears and is enabled if (valid number is present in the search bar) AND (connected devices with sms capab is more than ZERO): Tooltip says what it says right now
P2. The "+New" button appears but is greyed if (valid number is present in search bar) AND (connected devices equal to ZERO): Tooltip says "No Devices Connected"
P3. The "+New" button doesn't appear otherwise, i.e. if (number in search bar is not valid).

Additional Enhancements:

A1. "+New" changes to "Open" if the number is already present in sms list

Questions: 
Q1. Is "+" necessary in "+New"?
Comment 10 himcesjf.prog 2023-08-17 21:32:18 UTC
In kdeconnect-kde/smsapp/qml/ConversationList.qml, the +New button requires SmsHelper.isAddressValid() to be true for the button to be enabled. The SmsHelper.isAddressValid() function can only be called when there is text in the input field. If there is no text in the input field, then the function will not be called, and the +New button will remain disabled.

So +New button is enabled when the following conditions are met:
1. There is text in the search field.
2. The text in the search field is a valid phone number (minimum of 3 digits and maximum upto 15 digits)
3. The device is connected.

Hence, the observed result of +New button appeared greyed out upon opening KDE Connect > SMS Messages as there is no text in the input field by default upon opening.


For the expected result of 'When Opening KDE Connect > SMS Messages +New button is available and can create new SMS Message' --

1. +New button to be available

onEnabledChanged: {
    enabled = true
}

//we want the button to be enabled whenever the application is opened, regardless of whether the device is connected or not. So, we use the onEnabledChanged() signal to set the enabled property of the button to true. Did not use onCreated() signal as the button would only be enabled once, when the application is first opened. If the device is not connected when the application is first opened, the button would be disabled.

2. Now that the button is enabled upon opening, too allow creating SMS when the device is connected, the number is valideate SMS. 

onClicked: {
    if (deviceConnected && SmsHelper.isAddressValid(filter.text)) {
        conversationListModel.createConversationForAddress(filter.text)
        view.currentIndex = 0
    }
}
Comment 11 Simon Redman 2023-08-19 11:48:10 UTC
@himcesjf

If you have a code change, feel free to make a merge request and we can discuss on the MR itself. If I understand the suggestion in your comment (#10) then it seems like it's just forcing enabled to be true always, which defeats the point of having validation.

In general I'm not happy with how this button works. I think the only quick fix here is to update the tooltip to tell users to enter a phone number in the search box. A bigger fix would be to create a new .qml view, similar to the view you get when you click the "New Conversation" in Google messages, which allows users to search their contacts as well as enter a number, and change the behavior of this button to open that view.