Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Network improvements #1365

Merged
merged 21 commits into from
Jun 26, 2024
Merged

Network improvements #1365

merged 21 commits into from
Jun 26, 2024

Conversation

teclator
Copy link
Contributor

@teclator teclator commented Jun 21, 2024

Description

Network was one of the first components we added to Agama and since then it the components have evolve a lot being one of the most out of date ones. Therefore it was known it deserve some love... and we tried to start revamping it a little bit.

Changes done:

  • Modified the Network Page using routes instead of popup modals to configure the connections or to connecto a WiFi network.
  • Modified the wifi selection list and added a drawer for showing the selected network description and connection options.
  • Notify device state reason changes in order to detect an authentication error when connecting to a WiFi (reopen the form is still pending)
  • Fixed gateway4 set up.
  • Do not show externally configured connections (libvirt, vpns..)

Manually tested

Pending:

  • Try to improve the state management as network changes usually close de opened websocket opening the door to miss state changes notifications. Because of that, the read of the devices, connections and WiFi networks is done everytime insted of modifying the current state with just the changes.
  • Show the form of the failing authentication WiFi.
  • Fix the unit tests.
  • Remove unnecesary files, and fix types descriptions.

Old UI

Screenshot from 2024-06-26 10-41-09
Screenshot from 2024-06-26 10-41-12
Screenshot from 2024-06-26 10-41-21
Screenshot from 2024-06-26 10-41-33

New UI

Screenshot from 2024-06-26 10-49-08
Screenshot from 2024-06-26 10-49-19
Screenshot from 2024-06-26 10-49-22
Screenshot from 2024-06-26 10-49-34
Screenshot from 2024-06-26 10-49-40

@coveralls
Copy link

Coverage Status

coverage: 70.879% (-0.02%) from 70.896%
when pulling 72abba0 on network_improvements
into be7223d on master.

@imobachgs imobachgs added this to the Agama 9 milestone Jun 25, 2024
@coveralls
Copy link

Coverage Status

coverage: 70.928% (-0.02%) from 70.945%
when pulling f7334f1 on network_improvements
into 7f641e4 on master.

@coveralls
Copy link

Coverage Status

coverage: 70.99%. remained the same
when pulling ca826a8 on network_improvements
into ce30ede on master.

teclator and others added 14 commits June 26, 2024 10:24
Co-authored-by: Knut Anderssen <kanderssen@suse.com>
Co-authored-by: Knut Anderssen <kanderssen@suse.com>
This commit includes two major changes:

  - To avoid an infinite loop when fetching network data by making use
    of React.useCallback.
  - To persist the selected WiFi network to localStorage to mitigate an
    UI glitch due to the fact that the whole component gets unmounted
    on websocket reconnection.
No matter if user accepts or cancel the edition, the router should
navigate back to the previous route instead to the parent route.
@teclator teclator changed the title [WIP] Network improvements Network improvements Jun 26, 2024
@teclator teclator marked this pull request as ready for review June 26, 2024 09:25
@coveralls
Copy link

Coverage Status

coverage: 71.01%. remained the same
when pulling 6fed0a0 on network_improvements
into 3428c4e on master.

@coveralls
Copy link

Coverage Status

coverage: 71.015%. remained the same
when pulling cba892a on network_improvements
into b8e9ab2 on master.

@coveralls
Copy link

Coverage Status

coverage: 70.945% (-0.07%) from 71.015%
when pulling cba892a on network_improvements
into b8e9ab2 on master.

teclator and others added 2 commits June 26, 2024 11:42
Co-authored-by: David Díaz <1691872+dgdavid@users.noreply.github.com>
const [devices, setDevices] = useState(undefined);
const [selectedConnection, setSelectedConnection] = useState(null);
const [devices, setDevices] = useState(initialDevices);
const [updateState, setUpdateState] = useState(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is updateState used? Perhaps a better name could help to know the intention.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was updateNetworks, as the method was get from WifiSelector and updateDevicesConnections was like too much.. was not very inspired at the moment of writing it :P

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed offline, it is wrong because we are not checking the updateState value.

<CardField>
<CardField.Content>
<EmptyState title={_("No WiFi support")} icon="wifi_off" color="warning-color-200">
{_("The system does not support WiFi connections, probably because of missing or disabled hardware.")}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put in the shoes of an inexperienced user... I do not know if "missing hardware" is good enough.

web/src/components/network/NetworkPage.jsx Outdated Show resolved Hide resolved
web/src/components/network/WifiNetworkListItemPage.jsx Outdated Show resolved Hide resolved
web/src/components/network/WifiNetworkListItemPage.jsx Outdated Show resolved Hide resolved
web/src/components/network/WifiNetworkListItemPage.jsx Outdated Show resolved Hide resolved
web/src/components/network/WifiNetworksListPage.jsx Outdated Show resolved Hide resolved
web/src/components/network/WifiSelectorPage.jsx Outdated Show resolved Hide resolved
web/src/components/network/WifiSelectorPage.jsx Outdated Show resolved Hide resolved
@coveralls
Copy link

Coverage Status

coverage: 70.945% (-0.07%) from 71.015%
when pulling 654e86c on network_improvements
into b8e9ab2 on master.

@coveralls
Copy link

Coverage Status

coverage: 70.945% (-0.07%) from 71.015%
when pulling 654e86c on network_improvements
into b8e9ab2 on master.

@coveralls
Copy link

Coverage Status

coverage: 70.945% (-0.07%) from 71.015%
when pulling d7b1a11 on network_improvements
into b8e9ab2 on master.

@imobachgs imobachgs merged commit f003fb4 into master Jun 26, 2024
3 checks passed
@imobachgs imobachgs deleted the network_improvements branch June 26, 2024 11:34
@coveralls
Copy link

Coverage Status

coverage: 70.945% (-0.07%) from 71.015%
when pulling c2340b7 on network_improvements
into b8e9ab2 on master.

@coveralls
Copy link

Coverage Status

coverage: 70.945% (-0.07%) from 71.015%
when pulling c2340b7 on network_improvements
into b8e9ab2 on master.

This was referenced Jun 26, 2024
imobachgs added a commit that referenced this pull request Jun 27, 2024
Prepare for releasing Agama 9. It includes the following pull requests:

- #1101
- #1202
- #1228
- #1231
- #1236
- #1238
- #1239
- #1240
- #1242
- #1243
- #1244
- #1245
- #1246
- #1247
- #1248
- #1249
- #1250
- #1251
- #1252
- #1253
- #1254
- #1255
- #1256
- #1257
- #1258
- #1259
- #1260
- #1261
- #1264
- #1265
- #1267
- #1268
- #1269
- #1270
- #1271
- #1272
- #1273
- #1274
- #1279
- #1280
- #1284
- #1285
- #1286
- #1287
- #1288
- #1289
- #1290
- #1291
- #1292
- #1293
- #1294
- #1295
- #1296
- #1298
- #1299
- #1300
- #1301
- #1302
- #1303
- #1304
- #1305
- #1306
- #1307
- #1308
- #1309
- #1310
- #1311
- #1312
- #1313
- #1314
- #1315
- #1316
- #1317
- #1318
- #1319
- #1320
- #1321
- #1322
- #1323
- #1324
- #1325
- #1326
- #1328
- #1329
- #1331
- #1332
- #1334
- #1338
- #1340
- #1341
- #1342
- #1343
- #1344
- #1345
- #1348
- #1349
- #1351
- #1352
- #1353
- #1354
- #1355
- #1356
- #1357
- #1358
- #1359
- #1360
- #1361
- #1362
- #1363
- #1365
- #1366
- #1367
- #1368
- #1371
- #1372
- #1374
- #1375
- #1376
- #1379
- #1380
- #1381
- #1383
- #1384
- #1385
- #1386
- #1387
- #1388
- #1389
- #1391
- #1392
- #1394
- #1395
- #1397
- #1398
- #1399
- #1400
- #1403
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants