This adds a simple checkbox, which will create and stop hotspot. Hotspot can be also
configured from the KCM, where users can choose a name and password.
BUG: 413323
ngraham | |
apol |
Plasma | |
VDG |
This adds a simple checkbox, which will create and stop hotspot. Hotspot can be also
configured from the KCM, where users can choose a name and password.
BUG: 413323
Automatic diff as part of commit; lint not applicable. |
Automatic diff as part of commit; unit tests not applicable. |
There are two things which need work or I'm not sure about:
Cool feature!
applet/contents/ui/Toolbar.qml | ||
---|---|---|
134 | is it always prossible to create the hotspot? I'd expect it not to be possible at all sometimes (visible should be false then) and if the wifi is in use it might not be able to do it? (enabled false then). |
Nice feature.
UI-wise, this would probably be better off as a button than a checkbox. Creating a hotspot is an action, and action == button. Once the hotspot has been created, the button text can change from "Create Hotspot..." to "Disable Hotspot". This is a bit nonstandard HIG-wise, but I think it's appropriate rnough here.
If you need an icon, please file a bug in Breeze | Icons and we'll get on it ASAP. Even if the icon doesn't get in in time for Plasma 5.18, having an iconless button isn't the end of the world.
@apol You are right, it is not possible to create a hotspot if one is already connected to a WiFi network. Currently enabling hotspot disables your previous connection (WiFi) and hides available WiFi networks.
UI-wise, this would probably be better off as a button than a checkbox.
Personally, I'm more inclined to a checkbox. Why is "turn wifi on/off" not an action? Right now, I don't see the big difference.
Is the new "hotspot" equivalent to the "shared wifi", you can already set up?
If so, without really reading the patch details yet, just some remarks or ideas:
Is this also true if one has two wifi network cards? IIRC I could run both with an additional usb netwok card simultaneously.
@jgrulich do you intend to create another patch to display all connected clients, similiar to the list of active connections? :)
That's a question. Do we want to allow to create a hotspot if users are already connected to w WiFi network? You are allowed to do this for example on Android. Users might not realize why they are not allowed to create a hotspot and disconnecting from WiFi doesn't seem to me be a good first step.
This is meant to be as simple as possible, allowing users to create a hotspot without needing them to know the details. On Android you also don't get to choose.
- What do you think about adding a small configure button for the default hotspot directly on the plasma-nm frontend next to the hotspot checkbox/button?
- That would make it much easier for newcomers to use and configure their hotspot without first digging in the KCM.
- I also think that a hotspot (shared wifi) connection should be better separated from the other wifi connections in the KCM. If I create a new hotspot in the KCM now, it hides in the long list of known connections.
I will probably use a new icon for this. I will need to request it.
- That's somehow also true for the plasma-nm list of available networks: It's not really clear that a hotspot in the list is a indeed hotspot but not another access point.
Same here. A different icon should be enough.
Is this also true if one has two wifi network cards? IIRC I could run both with an additional usb netwok card simultaneously.
In case of multiple wifi cards we will choose the one which is not in use. Still any wifi card makes this option available, even if it's in use.
@jgrulich do you intend to create another patch to display all connected clients, similiar to the list of active connections? :)
I don't think NetworkManager provides this information.
applet/contents/ui/Toolbar.qml | ||
---|---|---|
134 | See Component.onCompleted: { }. We make this visible based on handler.hotspotSupported, which checks NM version and whether there is available wifi card. It's done this way because it's a non-notifyable property. |
I used ToolButton instead of a regular button, reason is that I don't think there is enough space. If there is modem device available, there will be three checkboxes on the left and with a regular button in various languages this might not be enough space.
This no longer compiles for me:
/home/nate/kde/src/plasma-nm/libs/handler.cpp: In member function ‘void Handler::createHotspot()’: /home/nate/kde/src/plasma-nm/libs/handler.cpp:622:94: error: ‘addAndActivateConnection2’ is not a member of ‘NetworkManager’; did you mean ‘addAndActivateConnection’? 622 | QDBusPendingReply<QDBusObjectPath, QDBusObjectPath, QVariantMap> reply = NetworkManager::addAndActivateConnection2(connectionSettings->toMap(), wifiDev->uni(), QString(), options); | ^~~~~~~~~~~~~~~~~~~~~~~~~ | addAndActivateConnection
No, even with networkmanager-qt from git master, I still get the same build failure here. :(
That's weird, because you can see here [1] that it's definitely there.
[1] - https://cgit.kde.org/networkmanager-qt.git/commit/?id=72a30f13c5ceac3e1b76d23f15d551d3689d3e15
Can someone please look into this review? Either try it or check the code? I would like to have this as part of Plasma 5.18 and deadline for that is in few days. @ngraham how is it going with the icon?
Leaving the user without a connection would be quite dishearting and possibly confusing.
I can imagine the use-case of: I am in this building with weird authentication (thinking university) and I want to share that connection with my phone, so I create a hotspot and we don't have to do weird configuration on the phone. Would apply to e.g. vpns too.
@cblack is working on it IIRC. It's too late to show the icon in Plasma 5.18 anyway (the icon will be in the breeze-icons repo which is a framework, and Frameworks 5.66 which Plasma 5.18 relies on has already been tagged. So there's no huge rush there IMO.
I'm still unable to test this because of the odd build failure that I cannot understand, explain, or overcome. :/
What about allowing to create a hotspot if the WiFi is in use, but it's not used as primary connection (e.g. Ethernet or Mobile connections are used primarily). In that case the user wouldn't be without connection if we disconnect him.
Then I'm not sure what to use instead. If I use a regular button with text, it won't definitely fit there with some translations. In Czech for example it would be "Vytvořit přístupový bod" which takes quite a lot of space, thus I would prefer just a button with an icon. If the icon is created, we can ship it together with plasma-nm for this release.
Is is possible to check if a hotspot icon exists, and if it does, show it, otherwise show a text?
@meven But we already know it's too late for the icon, at least for distros that won't update to KF5 5.67 before Plasma 5.18. Are you saying this can't go forward without an icon?
If we use an icons-only button/checkbox, then this can't go in without the icon, which means it's deferred to Plasma 5.19. There's still (barely) time to get it into 5.18 if we make sure that we use a UI that has both icons and text.
What about allowing to create a hotspot if the WiFi is in use, but it's not used as primary connection (e.g. Ethernet or Mobile connections are used primarily). In that case the user wouldn't be without connection if we disconnect him.
Surely. And if the laptop is capable of sharing a wifi connection through a hotspot (or through ethernet), that should be possible as well.
Address review comments: