search

Floogen / Stardrop

Stardrop is an open-source, cross-platform mod manager for the game Stardew Valley.
https://floogen.gitbook.io/stardrop/
GNU General Public License v3.0
114 stars 31 forks source link

Convert column search filters `ComboBox` to `ListBox` inside a `Flyout` #154

Closed Adda0 closed 7 months ago

Adda0 commented 8 months ago

This PR implements the multi-select feature for column search filters. It enables opening a flyout with column search filters, where one can select multiple filters.

This PR serves as an initial implementation of the functionality for filtering on multiple columns, as discussed in https://github.com/Floogen/Stardrop/pull/143#pullrequestreview-1799531288. Option to set default filters in the settings menu is currently missing and is WIP.

Styling for the UI elements is missing. If you like the implementation, I would appreciate it if you could apply suitable styles to the added Flyout and ListBox elements.

What do you think about this approach, functionality-wise? I think this is simpler than adding an arbitrary list with checkboxes for each column filter, which would have to be each manually handled. If you find this interesting, feel free to modify the code as you see fit.

The PR was tested on Linux, not tested on Windows and MacOS.

Floogen commented 7 months ago

This is exactly what I had in mind! I implemented the styling and adjusted the display text to dynamically show the amount of filters selected.

If it looks good to you, I will go ahead and merge this over.

Adda0 commented 7 months ago

It looks great. Thank you for adding styling. I am OK with merging this PR.

After this PR is merged, I will rebase the flatpak to build from the current upstream development branch. Afterwards, I can start polishing the flatpak and resolving the standing issues with the current flatpak build, in order to (optionally) convert the unofficial flatpak into an official one when everything is resolved.

Furthermore, Avalonia does not support Wayland as of now. Support for Wayland should be added in Avalonia 11 (the current Avalonia version) in the foreseeable future. Seeing as the whole Linux ecosystem is swiftly migrating over to Wayland (and using X causes issues in the Flatpak build), I would like to support Wayland in Stardrop as well. The first step would be to update Stardrop to Avalonia 11. I managed to get Stardrop working on Avalonia 11. Only the styling is broken as of now. I will rebase this branch onto the current upstream development branch and see why the styling does not work. This update could come as another PR, if you agree to the proposed approach.

Floogen commented 7 months ago

Merged!

I had looked into Avalonia 11 a while back, but the benefits of upgrading did not outweigh having to handle the breaking changes for the project. If you already have Stardrop successfully migrated over, I will gladly accept a PR for it (I don't mind looking into the styling issue either).

Adda0 commented 7 months ago

The flatpak has been rebased onto the current development branch. It is now running the unmodified upstream version of Stardrop. The only changes that remain are for resolving issues with Flatpak. Therefore, I will try to resolve the remaining issues and workarounds for the Flatpak version. I will probably create a few PRs, each for one workaround or issue. The PRs can either be merged, or closed and I will keep them as a patch for the Flatpak version.

I had looked into Avalonia 11 a while back, but the benefits of upgrading did not outweigh having to handle the breaking changes for the project. If you already have Stardrop successfully migrated over, I will gladly accept a PR for it (I don't mind looking into the styling issue either).

I agree. But Wayland support is crucial for Linux and Flatpak. Therefore, I will have to move to Avalonia 11 at some point in the future, at least on the fork. The changes necessary were surprisingly negligible. Henceforth, I will now work on trying to rebasing the Avalonia 11 branch onto the current development branch and create a PR for it.