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.

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

feat(ui): Implement RoomList::entries_with_dynamic_filter #2392

Merged
merged 12 commits into from
Aug 10, 2023

Conversation

Hywan
Copy link
Member

@Hywan Hywan commented Aug 9, 2023

Address #1911.

Thanks for @jplatte for his collaboration on this patch.

This patch can be reviewed commit-by-commit, but a one-diff review is also doable I guess.

On FFI side:

  • RoomList::entries is now infallible,
  • RoomList::entries_with_dynamic_filter has been implemented. It returns a stream + a RoomListDynamicFilter object, which has a single method: set. It accepts RoomListDynamicFilterKind value, which is an enum with All or FuzzyMatchRoomName variants.

Why this design? Because it allows the client-app to use entries if it provides a single “static” room list, or entries_with_dynamic_filter if it considers to filter the list with various filters. The client-app doesn't have to call entries or entries_with_dynamic_filter alternatively, it can just switch between All or other variants depending of the filters.

On the Rust side:

  • This patch implements the new all filter, which returns true for all RoomListEntry that are Filled or Invalidated,
  • This patch implements the new RoomList::entries_with_dynamic_filter method,
  • This patch renames the RoomList::entries_filtered method to ::entries_with_static_filter.

Hywan and others added 11 commits August 9, 2023 10:01
This patch updates the tests to ensure `entries_with_dynamic_filter`
works as expected.
These test cases are more easy to understand than the previous one if
you ask me.
First, this patch makes `RoomList::entries` infallible.

Second, this patch implements `RoomList::entries_with_dynamic_filter`.
This patch implements a new `all` filter.
This patch removes
`RoomListEntriesDynamicFilter::set_with_fuzzy_match_pattern` and
replaces it by a single `::set` method. It takes a new enum as
parameter: `RoomListEntriesDynamicFilterKind`. This enum has a
`FuzzyMatchRoomName` variant to simulate the removed method. It also
adds the `All` variant, to represent the `all` filter.
@Hywan Hywan requested a review from a team as a code owner August 9, 2023 13:09
@Hywan Hywan requested review from poljar and removed request for a team August 9, 2023 13:09
@Hywan Hywan requested a review from bnjbvr August 9, 2023 13:33
@codecov
Copy link

codecov bot commented Aug 9, 2023

Codecov Report

Patch coverage: 92.59% and project coverage change: +0.01% 🎉

Comparison is base (80c9464) 77.78% compared to head (0f94f93) 77.79%.
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2392      +/-   ##
==========================================
+ Coverage   77.78%   77.79%   +0.01%     
==========================================
  Files         175      176       +1     
  Lines       18619    18643      +24     
==========================================
+ Hits        14482    14504      +22     
- Misses       4137     4139       +2     
Files Changed Coverage Δ
...matrix-sdk-ui/src/room_list_service/filters/mod.rs 100.00% <ø> (ø)
crates/matrix-sdk-ui/src/room_list_service/mod.rs 94.44% <50.00%> (-1.02%) ⬇️
...s/matrix-sdk-ui/src/room_list_service/room_list.rs 98.03% <95.00%> (-1.97%) ⬇️
...matrix-sdk-ui/src/room_list_service/filters/all.rs 100.00% <100.00%> (ø)
...room_list_service/filters/fuzzy_match_room_name.rs 100.00% <100.00%> (ø)
crates/matrix-sdk/src/sliding_sync/list/mod.rs 90.38% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

/// Create a new filter that will accept all filled or invalidated entries.
pub fn new_filter() -> impl Fn(&RoomListEntry) -> bool {
|room_list_entry| -> bool {
matches!(room_list_entry, RoomListEntry::Filled(_) | RoomListEntry::Invalidated(_))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I find it weird that this filter is called "all" but still excludes some entries.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that’s a technical details. Empty are always filtered out because they are never rendered.

@Hywan Hywan merged commit 51c25a4 into matrix-org:main Aug 10, 2023
38 checks passed
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.

2 participants