Skip to content

refactor(web): improve appearance and performace of DASD page #2648

Merged
ancorgs merged 33 commits intomasterfrom
dasd-page-refactoring
Aug 20, 2025
Merged

refactor(web): improve appearance and performace of DASD page #2648
ancorgs merged 33 commits intomasterfrom
dasd-page-refactoring

Conversation

@dgdavid
Copy link
Copy Markdown
Contributor

@dgdavid dgdavid commented Aug 8, 2025

Problem

Some users have reported that the DASD page hangs for them, although it hasn't be reproducible by the Agama team members until now.

Solution

Although the issue couldn't be reproduced, the entire page has been reimplemented to use the same component that the main storage page uses for creating the device selector. This change helps reduce duplicated code, brings the page up to date, and increases the chances of catching bugs earlier in the shared component.

In addition to the component migration, several usability improvements have been made:

  • Reduced nesting, more horizontal space
  • Added new filtering options
  • Hints to help users discover bulk actions, now as control buttons instead of a selector
  • Ability to apply actions directly to individual devices without selecting them first
  • UI is now blocked while actions are in progress, preventing interactions with potentially stale data

Screenshots

Click to show/hide some screenshots

Note: these screenshots might be outdated. They will not be updated during code review unless the interface changes drastically due to requested changes.

  • General view
localhost_8080_ (31)
  • Actions per device/row
localhost_8080_ (38)
  • Filtering without results
localhost_8080_ (39)
  • Actions in progress
localhost_8080_ (37)
  • Format modals
localhost_8080_ (36) localhost_8080_ (34) localhost_8080_ (33) localhost_8080_ (32)

dgdavid added 2 commits August 7, 2025 16:25
In the latest version of the Agama interface, Page.Sections elements are
used to wrap specific regions of a page, not the entire page itself.

This commit removes an outdated full-page wrapper at DASD page to align
with that practice.
Ensures that item deselection works even when the deselected object is
equal in value but not the same instance (i.e. different memory
reference).

This can happen when operations like sorting produce a new list
containing objects with the same content but different references.
Previously, strict reference comparison prevented proper deselection in
these cases.

Now component uses Radashi's `isEqual` method, which relies on
same-value equality using `Object.is()`[1], to correctly identify
selected items.

[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Equality_comparisons_and_sameness#same-value_equality_using_object.is
@coveralls
Copy link
Copy Markdown

coveralls commented Aug 8, 2025

Coverage Status

coverage: 64.192% (+0.1%) from 64.094%
when pulling 4cbcc30 on dasd-page-refactoring
into 1f61cb5 on master.

This makes it possible to use the centralized table wrapper with tables
that require sorting behavior, such as the one listing DASD devices.

By centralizing table logic, we ensure consistent look & feel, improve
code reuse, and automatically benefit from enhancements applied to the
shared component.
Enables selecting or unselecting all items at once when using the
"multiple" selection mode, by rendering a checkbox in the selection
column header.
@dgdavid dgdavid force-pushed the dasd-page-refactoring branch from 6f9c584 to 8f61fb3 Compare August 11, 2025 08:13
dgdavid added 16 commits August 11, 2025 16:12
Add the `itemEqualityFn` prop to core/SelectableDataTable to allow
custom logic for comparing items during selection. This helps avoid
issues with deep comparisons that might fail when a property changes
in an object while the previous value is still held in the selected
state.

It also supports cases where items don't have a stable ID or require
alternative comparison logic.

By default, the component compares items using the `itemIdKey`
(defaults to `"id"`). If the key is missing, strict equality (`===`)
is used as a fallback.
Replaces the custom table implementation with the generic
SelectableDataTable component, preserving the existing structure and
behavior.

This change removes the complex logic that handled selection persistence
across filtering. That approach was not only hard to maintain but also
introduced cognitive overhead and edge cases. Instead, selection is now
cleared when filters are applied, leading to simpler, more predictable
behavior.
Since returning "void" does not fullfill the requirement of return a
ReactNode.
Adds optional support for displaying per-row action menus in the
SelectableDataTable component. When provided, an additional column is
rendered containing a menu with actions specific to each item.
By using Readonly and Pick utility types to avoid duplicating definition
and documentation of some properties.
Moved device actions to a toolbar above the table that only shows up
when there’s at least one device selected. A hint is shown when nothing
is selected to let users know actions will be available there. This
replaces the former "Perform an action" menu, which was disabled until
something was selected and could be confusing at first glance. This
should improve usability by making all available actions visible upfront
and saving clicks.

Additionally, per-device actions are now accessible via an actions menu
in the "Actions" column, making it easier to perform actions
individually.

A better code organization is expected in upcoming commits.
Replaces multiple local useState hooks (filters, sorting, selection, and
formatting) with a unified useReducer setup. This centralizes state
management, improves readability, and sets the stage for future
enhancements like combined filters or reset behavior.

Also renames `formatRequested` to `devicesToFormat`, merging it with
former `selectedDevices` to avoid maintaining redundant
selection-related state.
Adds a missing `return` keyword in the value function for the "DIAG"
column, which prevented proper rendering of "Yes"/"No" labels.

Also improves the overall documentation for DASD table columns.
Adds support for filtering DASD devices by their current status. Also
refactors the filtering logic to support combining multiple filters and
resets the device selection on each filter change to avoid stale
selections.
Allows users to filter devices by whether they are formatted,
unformatted, or both. It should help to improves usability when managing
large sets of DASD devices.
Improves clarity, accessibility, and reusability by extracting shared
filter components. Adds missing unit tests.
Refactor how some internals are built to reduce code duplication and
declutter the DASDTable component a bit.
Adds a new `emptyState` prop that allows rendering custom content when
the items array is empty. The empty state spans all columns (including
selection and actions, if enabled).
Renders a friendly interface when the DASD device list is empty,
including a custom icon, explanation message, and a "Clear all filters"
action.
@dgdavid dgdavid force-pushed the dasd-page-refactoring branch from 4cbcc30 to 92fbdce Compare August 17, 2025 08:57
Distinguish between when there are no DASD devices in the system and
when no devices match the applied filters. This provides more accurate
feedback to the user in the empty state UI.
But not if there are no items to show/select/unselect.
* data across nested calls.
*
* - `rowIndex` is intentionally mutable and is updated as rows are rendered.
* - `sortedBy` and `updateSorting` are read-only references to external sorting state.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There are now more read-only props, refactor that comment.

<Popup isOpen title={sprintf(_("Format device %s"), device.id)}>
<Content>
<Stack hasGutter>
<Text isBold>{_("This action could destroy any data stored on the device.")}</Text>
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is the original message was there before starting this refactorization, but I'd say it is "will" not "could".

type DASDTableAction =
| { type: "UPDATE_SORTING"; payload: DASDTableState["sortedBy"] }
| { type: "UPDATE_FILTERS"; payload: DASDTableState["filters"] }
| { type: "RESET_FILTERS" }
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Maybe "CLEAR_FILTERS" is a better naming. Applies to all "reset_filters" and also to "reset_selection"

Tracks DASD devices undergoing async mutations using a new `waitingFor`
state property. A modal is shown to indicate progress and disappears
once all pending actions are complete. Listens for `DASDDeviceChanged`
events to update the state accordingly.

This should be considered a workaround to provide user feedback and
prevent further actions while operations are in progress, until a
more robust solution is implemented.
@teclator
Copy link
Copy Markdown
Contributor

In general LGTM, some of the issues found are not related to the current changes, so, please add the changelog entry and mark it as ready for review. I just fixed an issue when sorting which was produced by changes done by #2412.

@dgdavid dgdavid marked this pull request as ready for review August 18, 2025 13:04
value: (d: DASDDevice) =>
// Displays comma-separated partition info as individual lines using <div>
d.partitionInfo.split(",").map((d: string) => <div key={d}>{d}</div>),
sortingKey: "partitionInfo",
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@teclator, @ancorgs

Does it make sense to enable sorting in this column?

dgdavid

This comment was marked as resolved.

@dgdavid dgdavid requested a review from teclator August 19, 2025 08:14
@dgdavid

This comment was marked as outdated.

@dgdavid dgdavid changed the title refactor(web): improve DASD page appearance and performace refactor(web): improve appearance and performace of DASD page Aug 20, 2025
dgdavid and others added 3 commits August 20, 2025 12:21
Co-authored-by: Knut Alejandro Anderssen González <kanderssen@suse.de>
Co-authored-by: David Díaz <1691872+dgdavid@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@ancorgs ancorgs left a comment

Choose a reason for hiding this comment

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

I'm relying on the LGTM written by @teclator to approve this.

@ancorgs ancorgs merged commit 8db0ce2 into master Aug 20, 2025
8 of 10 checks passed
@ancorgs ancorgs deleted the dasd-page-refactoring branch August 20, 2025 15:33
bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Aug 21, 2025
https://build.opensuse.org/request/show/1300561
by user IGonzalezSosa + anag_factory
- Refactor DASD page to make it more usable and improve its
  performance (related to bsc#1247444, gh#agama-project/agama#2648).

- Enforce ESP partition id for /boot/efi partitions manually
  defined (bsc#1248189).

- Fix messages about automatic storage sizes
  (gh#agama-project/agama#2657).
dgdavid added a commit that referenced this pull request Sep 2, 2025
)

During the [work to improve the DASD
page](#2648), the core
component used to build rich tables was extended to support user-driven
sorting.

This makes it possible to add a small enhancement to the device selector
dialog on the storage page, allowing users to sort the available devices
by either **name** or **size**, useful when setting up the installation
in a system with an noticeable amount of disks.
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