Bug 390148

Summary: No feedback when I try to add an invalid source URI/URL
Product: [Applications] Discover Reporter: Patrick Silva <bugseforuns>
Component: discoverAssignee: Aleix Pol <aleixpol>
Severity: normal CC: nate
Priority: NOR Keywords: usability
Version: 5.12.0   
Target Milestone: ---   
Platform: Arch Linux   
OS: Linux   
Latest Commit: Version Fixed In:
Attachments: What the "Add source" dialog looks like now

Description Patrick Silva 2018-02-09 13:12:33 UTC
Discover shows no error message when I try add an invalid source uri/url.
Comment 1 Aleix Pol 2018-02-12 18:10:05 UTC
Where do you add it?
Comment 2 Nate Graham 2018-02-12 18:11:25 UTC
This is in the dialog that appears from Settings > hamburger dropdown menu for a backend > Add repo/source.
Comment 3 Patrick Silva 2018-02-12 18:14:45 UTC
(In reply to Nate Graham from comment #2)
> This is in the dialog that appears from Settings > hamburger dropdown menu
> for a backend > Add repo/source.

Comment 4 Aleix Pol 2018-02-26 15:48:15 UTC
Git commit c9bff67739e672454ec733377a64b0518c46fbdc by Aleix Pol.
Committed on 26/02/2018 at 15:43.
Pushed by apol into branch 'Plasma/5.12'.

Don't close the add source dialog until a proper source has been offered

M  +21   -6    discover/qml/AddSourceDialog.qml
M  +5    -0    libdiscover/backends/DummyBackend/DummyBackend.cpp
M  +1    -0    libdiscover/backends/DummyBackend/DummyBackend.h
M  +3    -0    libdiscover/backends/DummyBackend/DummySourcesBackend.cpp

Comment 5 Nate Graham 2018-02-27 22:53:56 UTC
This commit introduces a fairly significant usability regression: the pop-up's close button is non-functional, so there's no visible way to get rid of it!
Comment 6 Aleix Pol 2018-03-01 12:29:05 UTC
Git commit 2b9e1b5e1ebbfab384b3fe3febe63e0ef076c273 by Aleix Pol.
Committed on 01/03/2018 at 12:15.
Pushed by apol into branch 'Plasma/5.12'.

Add a close button to AddSourceDialog

M  +12   -0    discover/qml/AddSourceDialog.qml

Comment 7 Nate Graham 2018-03-01 15:07:07 UTC
I don't think that's the right fix. That commit adds a symbolic close button in the corner that does work (though it's blurry and pixellated for some reason), but the textual close button still does nothing. Let's fix the existing close button rather than leaving it broken and adding a redundant control.
Comment 8 Aleix Pol 2018-03-01 19:04:27 UTC
Can you show me what you see? I have no idea what you are talking about.
Comment 9 Nate Graham 2018-03-01 19:08:44 UTC
Created attachment 111117 [details]
What the "Add source" dialog looks like now

Sure, here's what it looks like now as of your latest commit.

The red symbolic close button that you just added works, but shouldn't even be there at all since there's a textual Close button below. Alas, the textual close button doesn't work; clicking on it does nothing.

Also, the "close" button should probably be renamed "cancel", neither button has an icon, and the return key doesn't invoke the OK button. All in all there's a lot of work to be done here, but we can track those with other bugs if you'd like.
Comment 10 Nate Graham 2018-03-03 23:06:28 UTC
I've submitted a patch that fixes all of the above-mentioned issues: https://phabricator.kde.org/D11003
Comment 11 Nate Graham 2018-03-06 12:29:03 UTC
Git commit b623a4afb12f0bfa74f76711598e84772a1c6d2f by Nathaniel Graham.
Committed on 06/03/2018 at 12:28.
Pushed by ngraham into branch 'master'.

Fix the Add Source dialog

Fix a variety of bugs and usability issues with the {nav Add Source} dialog:
- Make the close button actually close the dialog
- Give the text field focus so you can immediately start typing
- Make the return key push the Add button
- Remove needless close symbol in top-right corner
- Give the buttons icons
- Re-word the title and make it larger

Test Plan:


Tested with Flatpak backend:
- Invalid URLs are rejected
- Valid URLs that nonetheless don't point to a Flatpak repo trigger an error message
- The close button now closes the dialog
- The escape key still closes the dialog
- The return and enter keys press the {nav Add} button

Reviewers: #discover_software_store, apol

Reviewed By: #discover_software_store, apol

Subscribers: acrouthamel, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D11003

M  +22   -20   discover/qml/AddSourceDialog.qml