Skip to content

feat(web): add support for sorting devices in DeviceSelectorModal#2672

Merged
dgdavid merged 8 commits intomasterfrom
device-selector-sorting
Sep 2, 2025
Merged

feat(web): add support for sorting devices in DeviceSelectorModal#2672
dgdavid merged 8 commits intomasterfrom
device-selector-sorting

Conversation

@dgdavid
Copy link
Copy Markdown
Contributor

@dgdavid dgdavid commented Aug 22, 2025

During the work to improve the DASD page, 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.

Sorting by name asc Sorting y size desc
localhost_8080_ (40) localhost_8080_ (41)

@dgdavid dgdavid changed the title feat(web): add support for sortering to device selector feat(web): add support for sorting to device selector Aug 22, 2025
@dgdavid dgdavid force-pushed the device-selector-sorting branch from 386d466 to 3cd8b9d Compare August 22, 2025 08:08
@coveralls
Copy link
Copy Markdown

coveralls commented Aug 22, 2025

Coverage Status

coverage: 75.965% (+11.7%) from 64.265%
when pulling aa03572 on device-selector-sorting
into 75f1832 on master.

@dgdavid dgdavid changed the title feat(web): add support for sorting to device selector feat(web): add support for sorting devices in DeviceSelectorModal Aug 24, 2025
Replaces repeated use of fast-sort's method chaining syntax with a
wrapper that accepts the sort direction as a direct argument. This
should improve readability and make it easier to build sorting logic
dynamically, as done in components like DeviceSelectorModal and
DASDTable.
@dgdavid dgdavid force-pushed the device-selector-sorting branch from ce5112d to 6c1e966 Compare August 24, 2025 17:16
@dgdavid dgdavid marked this pull request as ready for review August 24, 2025 17:25
@dgdavid dgdavid requested a review from ancorgs August 24, 2025 17:25
And add testing for sorting feature.
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.

It generally looks good. Just a comment about inline documentation.

@dgdavid dgdavid requested a review from ancorgs September 2, 2025 07:59
@@ -1,7 +1,13 @@
-------------------------------------------------------------------
Tue Sep 2 08:24:38 UTC 2025 - David Diaz <dgonzalez@suse.com>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see you were cleaning surplus spaces. Great!
But I was actually expecting two spaces between "Sep" and "2" (I think that's the default in this format for days with just one digit). But I don't think that really matters.

Copy link
Copy Markdown
Contributor Author

@dgdavid dgdavid Sep 2, 2025

Choose a reason for hiding this comment

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

But I was actually expecting two spaces between "Sep" and "2"

Me too.

Originally I "kept" it. In fact, I added it because I solved the conflict manually using the Github web interface... but CI complained about wrong date and the only thing I found the extra space. I can add it back, though.

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.

LGTM

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.

Still LGTM after the latest merge. Hurry up and merge before another changelog conflict kicks in. :-)

@dgdavid dgdavid merged commit 7218579 into master Sep 2, 2025
21 checks passed
@dgdavid dgdavid deleted the device-selector-sorting branch September 2, 2025 14:20
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.

3 participants