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.

Sign up for GitHub

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

Jump to bottom

Adapt network to the new UI #1302

Merged
merged 2 commits into from
Jun 11, 2024
Merged

Adapt network to the new UI #1302

merged 2 commits into from
Jun 11, 2024

Conversation

ancorgs
Copy link
Contributor

@ancorgs ancorgs commented Jun 11, 2024 •
edited
Loading

This includes the pure UI changes:

  • Turning sections into cards
  • Moving the "Connect to Wi-Fi" button from the special menu to the header

network

But in my experience, there are quite some functional (beyond UI) aspects that are currently broken:

  • I would not expect two entries on "WiFi connections" since I only have one WiFi network card. Looks like all connections related to the card (both active and non-active) are displayed.
  • wifiScanSupported is always false, so I had to tweak some conditions to make the "connect" button visible at the screenshot above.
  • If I click on any of the available networks at the WiFi scanning pop-up, I get the following error.

cannot

dgdavid
dgdavid reviewed Jun 11, 2024
View reviewed changes
web/src/components/network/NetworkPage.jsx
<FlexItem>
<h2>{_("Network")}</h2>
</FlexItem>
<If
Copy link
Contributor

Choose a reason for hiding this comment

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

Note, I'm killing the <If> component too. There were no few complains about it in the past :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, another note: for the shake of reducing nodes, I'm using Flex with props when possible instead of Flex with FlexItems. But this is just a comment, please keep the change as it is.

dgdavid
dgdavid approved these changes Jun 11, 2024
View reviewed changes
Copy link
Contributor

@dgdavid dgdavid left a comment

Choose a reason for hiding this comment

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

Looks good, just a suggestions about the button look & feel. But feel free to merge it as it is since it is good enough for the task we're addressing this week.

web/src/components/network/NetworkPage.jsx Outdated
condition={wifiScanSupported}
then={
<FlexItem align={{ default: "alignRight" }}>
<Button onClick={openWifiSelector}>{_("Connect to a Wi-Fi network")}</Button>
Copy link
Contributor

Choose a reason for hiding this comment

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

What about using variant="secondary" here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@ancorgs ancorgs marked this pull request as ready for review June 11, 2024 09:46
@ancorgs ancorgs merged commit 8efa654 into agama-project:new-ui-proposal Jun 11, 2024
1 check failed
@imobachgs imobachgs mentioned this pull request Jun 12, 2024
Merged
dgdavid added a commit that referenced this pull request Jun 13, 2024
@dgdavid
feat(web): remake the user interface (#1202)
3df8883 
As Agama has progressed, the original idea of a hyper-minimalist
interface has completely vanished. We have moved far away from the first
_SPA interactive interface_. The installation summary has lost all its
value and it is now a sort of navigation menu or index. Furthermore, it
is impossible to start with a valid configuration straight away due to
several reasons, such as

* The creation of a user it's mandatory since it cannot be relegated to
the first boot.
* The storage proposal algorithm [does not perform as many
attempts](#1159 (comment))
as it used to do in YaST.

Last but not least, it is way weird landing in a page with a big, green,
and enabled <kbd>Install</kbd> button that will prompt an error when
clicked before any other user interaction.

There is a [proposal to
improve](#778) that first
initial screen, but having in mind the Agama development progression I
believe that the current approach does not scale. Regrettably, looks
like a dead end.

With this in mind, I have been thinking a bit about an alternative that
I had on my to-do list for the future, which consisted of converting the
summary screen into a panel on the left and loading the content of each
section to the right (which would overlap the first on small devices).
It would solve two problems in one shot by stop forcing the user to
navigate back and forth to change between sections and stop wasting
available space on large screens. The key was to make better use of
react-router and embrace [nested
routes](https://reactrouter.com/en/main/start/overview#nested-routes) as
designed instead of fighting against them. Something perfectly doable.

However, as soon as I started writing some code to play with, I realized
that also embracing more heavily PatternFly would be enough to start
making it possible. Moreover, I concluded that it could even help to
solve many of the problems we currently have with the interface at many
levels.

So I got to work to carry out a small proof of concept with, among
others, following ideas in mind,

* Use PatternFly as much as possible to the point of looking familiar
with Cockpit UI but keeping some bits of Agama's identity (like
typography and colors, the absence of wizard, to allow the user to move
as freely as possible, etc)
* Reduce the number of components developed by us and help to improve
the existing ecosystem instead. After all, at this moment it is not
realistic to think that we can also develop and maintain a design and
components system (although I would like to do so, of course :P).
* Sensibly embrace the router: use a [data
router](https://reactrouter.com/en/main/routers/picking-a-router#using-v64-data-apis),
[nested
routes](https://reactrouter.com/en/main/start/overview#nested-routes),
[outlet
context](https://reactrouter.com/en/main/hooks/use-outlet-context), etc.
* Use [FormData
API](https://developer.mozilla.org/en-US/docs/Web/API/FormData) when
working with forms and/or evaluate the use of react-router
[Form](https://reactrouter.com/en/main/components/form)
* Keep an eye on the [next React
version](https://react.dev/blog/2024/04/25/react-19).
* Keep in mind that the number of internal states of some components can
be reduced by relying in the [URL as State
Management](https://www.youtube.com/watch?v=VenLRGHx3D4&t=602s) when
possible.


Indeed, it is A LOT of work to do, but I firmly believe it worth. Once
we finish the migration, we should be able to move forward more
efficiently and, hopefully, with less friction when taking UI decisions.
Don't get me wrong, we will still have work to do, decisions to make,
and specially things to improve, etc. We will even keep changing our
minds from time to time based on learned lessons or feedback gathered.
But with a bit of luck, we will have more time for these things.

**PLEASE REMEMBER** there will be a lot of details to define and many
others to polish after this PR gets merge, but in general terms this
new, streamlined layout gives us more room for placing almost everything
you will miss at first sight. **Little by little, please.**

Bonus: PatternFly already provides style guides, and we can build our
own on top of them since it will be inevitable that, at certain points
and due to the nature of Agama and our view/knowledge, we will take
slightly different decisions/paths.

## Related pull requests

- #1298
- #1299
- #1300
- #1301
- #1302
- #1303
- #1304
- #1305
- #1306
- #1307
- #1308
- #1309
- #1310
- #1312
- #1313
- #1315
- #1316
- #1317
- #1319
- #1320
- #1321
- #1322
- #1323
- #1324
- #1325
- #1326
- #1328
@imobachgs imobachgs mentioned this pull request Jun 27, 2024
Merged
imobachgs added a commit that referenced this pull request Jun 27, 2024
@imobachgs
Update to release 9 (#1404)
8fe19db 
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
Reviewers

@dgdavid dgdavid dgdavid approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ancorgs @dgdavid