Do not show WiFi configuration link if there are no WiFi devices#374
Do not show WiFi configuration link if there are no WiFi devices#374
Conversation
teclator
changed the title
imobachgs
left a comment
imobachgs
left a comment
There was a problem hiding this comment.
Now that we have plenty of time, I would take the opportunity to improve a few things. For instance, if the network is not initialized, we display display nothing, and it says nothing to the user. Additionally, if you do not have any network device (e.g., the network cable is disconnected and you do not have any wireless device), you basically see an empty section, right?
teclator
force-pushed
the
wifi_devices
branch
2 times, most recently
from
3f4c0a2 to
f9af886
Compare
|
@imobachgs I addressed the suggestions from our previous comment, so, only deciding what to show in case that the section is completely empty.
|
I know we (the YaST team) haven't talk about that yet, but in my opinion the summary mustn't contain an empty section. The section can be there or not, depending on the - not yet ready - [product|architecture|workflow] configuration, but no empty. So, I'd add a summary telling the user that D-Installer was not able to find neither, a wired network connection nor a network hardware for connection to a WiFi network. |
teclator
force-pushed
the
wifi_devices
branch
2 times, most recently
from
613c7be to
751e76d
Compare
teclator
changed the title
teclator
changed the title
|
|
| /* | ||
| * Returns the list of WiFi devices available in the system | ||
| * | ||
| * @return {object[]} list of available WiFi devices | ||
| */ | ||
| availableWifiDevices() { | ||
| return Object.values(this.proxies.devices).filter(d => d.DeviceType === NM_DEVICE_TYPE_WIFI); | ||
| } | ||
|
|
||
| /* | ||
| * Returns whether the system is able to scan wifi networks based on rfkill and the presence of | ||
| * some wifi device | ||
| * | ||
| * @return {boolean} | ||
| */ |
There was a problem hiding this comment.
NP: looks to me that those comments are miss-aligned.
/*
*should actually be
/*
*|
|
||
| /* | ||
| * Returns NetworkManager general settings | ||
| * |
There was a problem hiding this comment.
NP: Ah, I see we have a mix on comments style :) So keep them as they are 🤷♂️
There was a problem hiding this comment.
I do not think these are mixed styles, just miss-aligned comments :) And sure we have more of them. 😞
dgdavid
left a comment
dgdavid
left a comment
There was a problem hiding this comment.
Do not forget to update the .changes file.
imobachgs
left a comment
imobachgs
left a comment
There was a problem hiding this comment.
Just a minor comment in the changes log. Otherwise, LGTM. Thanks!
4 participants
Problem
In #316 we added logic to hide the link for connecting to a WiFi network but only in case the WiFi was enable based on rfkill but if there are no WiFi devices at all then it does not makes sense to show the link.
Solution
Do not show the link for connecting to a WiFi network if there are no WiFi devices.
Update: and also do not show an empty network section when there is no connection detected and no WiFi device available.
Testing
Screenshots