Skip to content

Commit

Permalink
feat(web): remake the user interface (#1202)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dgdavid authored Jun 13, 2024
2 parents bc2c5d7 + 23a05f7 commit 3df8883
Show file tree
Hide file tree
Showing 222 changed files with 4,914 additions and 9,014 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci-web.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ jobs:
run: npm run stylelint

- name: Run the tests and generate coverage report
run: npm test -- --coverage
run: npm test -- --coverage --silent
#
# # send the code coverage for the web part to the coveralls.io
# - name: Coveralls GitHub Action
Expand Down
9 changes: 2 additions & 7 deletions doc/dbus/bus/org.opensuse.Agama.Users1.bus.xml
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,7 @@
<property type="s" name="RootSSHKey" access="read"/>
<property type="(sssba{sv})" name="FirstUser" access="read"/>
</interface>
<interface name="org.opensuse.Agama1.ServiceStatus">
<property type="aa{sv}" name="All" access="read"/>
<property type="u" name="Current" access="read"/>
</interface>
<interface name="org.opensuse.Agama1.Validation">
<property type="as" name="Errors" access="read"/>
<property type="b" name="Valid" access="read"/>
<interface name="org.opensuse.Agama1.Issues">
<property type="a(ssuu)" name="All" access="read"/>
</interface>
</node>
1 change: 0 additions & 1 deletion doc/dbus/bus/org.opensuse.Agama1.ServiceStatus.bus.xml

This file was deleted.

1 change: 0 additions & 1 deletion doc/dbus/bus/org.opensuse.Agama1.Validation.bus.xml

This file was deleted.

35 changes: 0 additions & 35 deletions doc/dbus/org.opensuse.Agama1.ServiceStatus.doc.xml

This file was deleted.

24 changes: 0 additions & 24 deletions doc/dbus/org.opensuse.Agama1.Validation.doc.xml

This file was deleted.

2 changes: 0 additions & 2 deletions doc/dbus_api.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ Service for managing storage devices.
.ObjectManager
.Agama1.ServiceStatus
.Agama1.Progress
.Agama1.Validation
.Agama.Storage1
.Agama.Storage1.Proposal.Calculator
.Agama.Storage1.ISCSI.Initiator
Expand All @@ -96,7 +95,6 @@ Service for managing storage devices.
.ObjectManager
.Agama1.ServiceStatus
.Agama1.Progress
.Agama1.Validation
.Agama.Storage1
.Agama.Storage1.Proposal.Calculator
.Agama.Storage1.ISCSI.Initiator
Expand Down
11 changes: 0 additions & 11 deletions rust/agama-lib/src/proxies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,14 +178,3 @@ trait Issues {
#[dbus_proxy(property)]
fn all(&self) -> zbus::Result<Vec<(String, String, u32, u32)>>;
}

#[dbus_proxy(interface = "org.opensuse.Agama1.Validation", assume_defaults = true)]
trait Validation {
/// Errors property
#[dbus_proxy(property)]
fn errors(&self) -> zbus::Result<Vec<String>>;

/// Valid property
#[dbus_proxy(property)]
fn valid(&self) -> zbus::Result<bool>;
}
12 changes: 12 additions & 0 deletions rust/agama-server/src/network/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,12 @@ async fn connect(
.await
.map_err(|_| NetworkError::CannotApplyConfig)?;

state
.network
.apply()
.await
.map_err(|_| NetworkError::CannotApplyConfig)?;

Ok(StatusCode::NO_CONTENT)
}

Expand All @@ -341,6 +347,12 @@ async fn disconnect(
.await
.map_err(|_| NetworkError::CannotApplyConfig)?;

state
.network
.apply()
.await
.map_err(|_| NetworkError::CannotApplyConfig)?;

Ok(StatusCode::NO_CONTENT)
}

Expand Down
6 changes: 3 additions & 3 deletions rust/agama-server/src/users/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use crate::{
error::Error,
web::{
common::{service_status_router, validation_router, EventStreams},
common::{issues_router, service_status_router, EventStreams},
Event,
},
};
Expand Down Expand Up @@ -125,7 +125,7 @@ pub async fn users_service(dbus: zbus::Connection) -> Result<Router, ServiceErro

let users = UsersClient::new(dbus.clone()).await?;
let state = UsersState { users };
let validation_router = validation_router(&dbus, DBUS_SERVICE, DBUS_PATH).await?;
let issues_router = issues_router(&dbus, DBUS_SERVICE, DBUS_PATH).await?;
let status_router = service_status_router(&dbus, DBUS_SERVICE, DBUS_PATH).await?;
let router = Router::new()
.route(
Expand All @@ -135,8 +135,8 @@ pub async fn users_service(dbus: zbus::Connection) -> Result<Router, ServiceErro
.delete(remove_first_user),
)
.route("/root", get(get_root_config).patch(patch_root))
.merge(validation_router)
.merge(status_router)
.nest("/issues", issues_router)
.with_state(state);
Ok(router)
}
Expand Down
9 changes: 9 additions & 0 deletions rust/agama-server/src/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,15 @@ async fn run_events_monitor(dbus: zbus::Connection, events: EventsSender) -> Res
)
.await?,
);
stream.insert(
"users-issues",
issues_stream(
dbus.clone(),
"org.opensuse.Agama.Manager1",
"/org/opensuse/Agama/Users1",
)
.await?,
);

tokio::pin!(stream);
let e = events.clone();
Expand Down
107 changes: 1 addition & 106 deletions rust/agama-server/src/web/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::{pin::Pin, task::Poll};
use agama_lib::{
error::ServiceError,
progress::Progress,
proxies::{IssuesProxy, ProgressProxy, ServiceStatusProxy, ValidationProxy},
proxies::{IssuesProxy, ProgressProxy, ServiceStatusProxy},
};
use axum::{extract::State, routing::get, Json, Router};
use pin_project::pin_project;
Expand Down Expand Up @@ -354,108 +354,3 @@ async fn build_issues_proxy<'a>(
.await?;
Ok(proxy)
}

/// Builds a router to the `org.opensuse.Agama1.Validation` interface of a given
/// D-Bus object.
///
/// ```no_run
/// # use axum::{extract::State, routing::get, Json, Router};
/// # use agama_lib::connection;
/// # use agama_server::web::common::validation_router;
/// # use tokio_test;
///
/// # tokio_test::block_on(async {
/// async fn hello(state: State<HelloWorldState>) {};
///
/// #[derive(Clone)]
/// struct HelloWorldState {};
///
/// let dbus = connection().await.unwrap();
/// let validation_routes = validation_router(
/// &dbus, "org.opensuse.HelloWorld", "/org/opensuse/hello"
/// ).await.unwrap();
/// let router: Router<HelloWorldState> = Router::new()
/// .route("/hello", get(hello))
/// .merge(validation_routes)
/// .with_state(HelloWorldState {});
/// });
/// ```
///
/// * `dbus`: D-Bus connection.
/// * `destination`: D-Bus service name.
/// * `path`: D-Bus object path.
pub async fn validation_router<T>(
dbus: &zbus::Connection,
destination: &str,
path: &str,
) -> Result<Router<T>, ServiceError> {
let proxy = build_validation_proxy(dbus, destination, path).await?;
let state = ValidationState { proxy };
Ok(Router::new()
.route("/validation", get(validation))
.with_state(state))
}

#[derive(Clone, Serialize, utoipa::ToSchema)]
pub struct ValidationResult {
valid: bool,
errors: Vec<String>,
}

async fn validation(
State(state): State<ValidationState<'_>>,
) -> Result<Json<ValidationResult>, Error> {
let validation = ValidationResult {
valid: state.proxy.valid().await?,
errors: state.proxy.errors().await?,
};
Ok(Json(validation))
}

#[derive(Clone)]
struct ValidationState<'a> {
proxy: ValidationProxy<'a>,
}

/// Builds a stream of the changes in the the `org.opensuse.Agama1.Validation`
/// interface of the given D-Bus object.
///
/// * `dbus`: D-Bus connection.
/// * `destination`: D-Bus service name.
/// * `path`: D-Bus object path.
pub async fn validation_stream(
dbus: zbus::Connection,
destination: &'static str,
path: &'static str,
) -> Result<Pin<Box<dyn Stream<Item = Event> + Send>>, Error> {
let proxy = build_validation_proxy(&dbus, destination, path).await?;
let stream = proxy
.receive_errors_changed()
.await
.then(move |change| async move {
if let Ok(errors) = change.get().await {
Some(Event::ValidationChanged {
service: destination.to_string(),
path: path.to_string(),
errors,
})
} else {
None
}
})
.filter_map(|e| e);
Ok(Box::pin(stream))
}

async fn build_validation_proxy<'a>(
dbus: &zbus::Connection,
destination: &str,
path: &str,
) -> Result<ValidationProxy<'a>, zbus::Error> {
let proxy = ValidationProxy::builder(dbus)
.destination(destination.to_string())?
.path(path.to_string())?
.build()
.await?;
Ok(proxy)
}
13 changes: 13 additions & 0 deletions rust/package/agama.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
-------------------------------------------------------------------
Thu Jun 13 10:50:44 UTC 2024 - Knut Anderssen <kanderssen@suse.com>

- Apply network changes when connecting or disconnecting
(gh#openSUSE/agama#1320).

-------------------------------------------------------------------
Thu Jun 13 10:39:57 UTC 2024 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>

- Expose Issues API in users-related interface
(gh#openSUSE/agama#1202).
- Drop the old validations API.

-------------------------------------------------------------------
Wed Jun 12 10:15:33 UTC 2024 - Jorik Cronenberg <jorik.cronenberg@suse.com>

Expand Down
48 changes: 0 additions & 48 deletions service/lib/agama/dbus/clients/with_validation.rb

This file was deleted.

Loading

0 comments on commit 3df8883

Please sign in to comment.