Add basic support for network configuration#260
Conversation
dgdavid
force-pushed
the
feature/network-overview
branch
2 times, most recently
from
60ff072 to
fa09b63
Compare
Co-Authored-By: Knut Anderssen <kanderssen@suse.com>
Co-Authored-By: Knut Anderssen <kanderssen@suse.com>
Co-Authored-By: Knut Anderssen <kanderssen@suse.com>
Let's start with a simple form in a popup until came with a better section/UI in the near future. Co-Authored-By: Knut Anderssen <kanderssen@suse.com>
To allow having "height-fixed" Popups, useful to avoid Popups chaging their size while adding or deleting dynamic elements, such addresses when configuring a wired connection.
Drops initial and incomplete support for ipv6 too.
By not displaying the connected or not connected "headers".
Although there is a lot of room for improvements in both, ready and pending tests. Something to be addressed in next iterations.
By providing all connection properties. Maybe some of them should be optional.
dgdavid
left a comment
dgdavid
left a comment
There was a problem hiding this comment.
Please, take this PR as a starting point for having a network overview and adding a basic support for its configuration. We know that there are plenty of improvements and nice features that can be added…. but we need to sort and planning them before continue to avoid still walking in circles 😉
| const activeWiredConnections = connections.filter(c => c.type === CONNECTION_TYPES.ETHERNET); | ||
| const activeWifiConnections = connections.filter(c => c.type === CONNECTION_TYPES.WIFI); | ||
|
|
||
| return ( |
There was a problem hiding this comment.
When there are none active connection
- Return an explanatory message (i.e., "Network cannot be configured", "No network connections available"), or
- Display a list of devices that can be configured to have an active connection
Something to do in next iterations, after having a more clear vision about the desired UI for that Network Overview component (please, try to avoid making it too complex; we could have a Network Section component for an advanced network setup).
| EOS_SOFTWARE as OverviewIcon, | ||
| EOS_TRANSLATE as LanguagesSelectionIcon, | ||
| EOS_VOLUME as HardDriveIcon, | ||
| EOS_SETTINGS_ETHERNET as NetworkIcon, |
There was a problem hiding this comment.
A better icon is more than welcome. But remember that we are using https://eos-icons.com/, so have a look there for consistence. (Spoiler: although it is listed there, EOS_LAN actually is not available; in fact, would be nice to use it to visually identify listed wired connections)
| <Button variant="link" onClick={open}> | ||
| { label } | ||
| <Button variant="link" onClick={open} isDisabled={ips.length === 1}> | ||
| {firstIp} {hostname && <Text component="small">({hostname})</Text>} |
There was a problem hiding this comment.
Despite we have a Network overview in the installation summary now, we have kept this component because it is interesting to have a way to quickly identify the machine we have dealing with. For example, think of someone performing multiple installations at the same time… they must be able to distinguish between these machines no matter which section is displayed (product selection, storage section, etc)
Of course, we can do a lot of things to improve the current presentation, but let's book some time to think/discuss about it later.
| .medium-modal-popup { | ||
| min-block-size: 55vh; | ||
| max-block-size: 55vh; | ||
| } | ||
|
|
||
| .large-modal-popup { | ||
| min-block-size: 85vh; | ||
| max-block-size: 85vh; |
There was a problem hiding this comment.
These height sizes were added to have height-fixed popups, which is useful to avoid having a popup that changes its height while adding or removing dynamic fields, as happens now with the Connection Settings.
If you think other sizes are more appropriated, please speak up 😉
dgdavid
changed the title
Configure nameservers
* Use the uuid (as id) and the id (as name) instead.
* Additionally, improve enums naming.
joseivanlopez
left a comment
joseivanlopez
left a comment
There was a problem hiding this comment.
LGTM
Very good work guys! Congrats.
BTW, having the list of IPs in the summary looks a bit weird, but I think we still have to refine the UI. So far so good. Thanks!
5 participants
Problem
So far, D-Installer does not offer a way to either, see how the network will end up configured or a way to change it.
Solution
Add a bare minimum and interactive summary in the installation overview.
We have been playing with several approaches (see one of them here) to be able to interact with the NetworkManager D-Bus API while keeping the summary simpler enough. Consequently, this first proposal only displays ETHERNET and WiFi active connections, allowing to change only the formers.
The basic support to interact with the wireless networks will be added as part of https://trello.com/c/SFpwJaU7/ (internal link).
Caveat
Of course, nothing is written in stone (yet :P) and this UI could be subject to change in favor of a better UX in the next iterations. Thus, take this PR as a starting point, which has helped a lot to understand how we want to interact with Network Manager DBus API and to have a big picture that will help continue the implementation of network features in D-Installer.
Testing
Screenshots
|