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

Room list search bar #232

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

tyreseluo
Copy link
Contributor

Fixes: #123

@tyreseluo tyreseluo changed the title Room list search bar WIP room list search bar Nov 5, 2024
@kevinaboos kevinaboos added the waiting-on-author This issue is waiting on the original author for a response label Nov 7, 2024
@tyreseluo tyreseluo marked this pull request as ready for review November 7, 2024 04:55
@kevinaboos kevinaboos removed the waiting-on-author This issue is waiting on the original author for a response label Nov 7, 2024
@tyreseluo tyreseluo changed the title WIP room list search bar Room list search bar Nov 7, 2024
@ZhangHanDong ZhangHanDong added the blocked-on-makepad Blocked on a Makepad bug or missing Makepad feature label Nov 7, 2024
Copy link
Member

@kevinaboos kevinaboos left a comment

Choose a reason for hiding this comment

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

The code structure itself looks good. I'd recommend that you think deeply about what kind of filter criteria we would want to accept (I left some suggestions), and also what kind of sorting criteria the user might care about, for example, by recency, alphabetical by name, etc. Then you need to consider how you could jointly consider different filter criteria and various sorting preferences.

src/home/rooms_sidebar.rs Outdated Show resolved Hide resolved
@@ -167,6 +175,25 @@ live_design! {
}
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub enum RoomsSideBarFilter {
Copy link
Member

@kevinaboos kevinaboos Nov 7, 2024

Choose a reason for hiding this comment

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

I think you're conflating filter kinds with room categories.... these are two separate things.

The filter categories would be things like:

  • search by room name
  • search by room ID
  • search by room member (user name and/or user ID)
  • search by anything (any keywords match)
  • etc

You could also have additional filter kinds for "is_direct" or "is_room" (not direct), but those wouldn't be expressible as enum variants because you'd only be able to select one. They would have to be bitflags/bitsets such that you could select multiple filter criteria at once. Or, alternatively, you could add a series of booleans to a "filter type" struct. The design is up to you, but try to think deeply about the possible valid combinations of filter kinds and search terms.

Copy link
Member

Choose a reason for hiding this comment

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

Other possible filter dimensions include whether a room is marked as a favorite, whether a user has left a room, joined a room, or is just invited to a room, etc.

The list of built-in filter functions are here; perhaps you could just re-use most of them. But note that we do not want to use them with the RoomListDynamicEntriesController because that modifies the actual room_list_service's list of rooms, which I don't want to deal with.

https://matrix-org.github.io/matrix-rust-sdk/matrix_sdk_ui/room_list_service/filters/index.html#functions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep. I got it. So i will do that and then change this issue's label. Thanks


self.display_filter = RoomDisplayFilter(Box::new(move |room| {
let room_name = room.room_name.as_ref().map(|n| n.to_lowercase());
room_name.as_ref().map_or(false, |n| n.contains(&keywords))
Copy link
Member

Choose a reason for hiding this comment

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

this is okay for now as a rudimentary search, but in the future we'd want to fuzzy match the room name, room ID, room members, etc against one or more of the keywords (rather than just a straight-up "contains keywords" conditional)

Co-authored-by: Kevin Boos <1139460+kevinaboos@users.noreply.github.com>
@kevinaboos kevinaboos added the waiting-on-author This issue is waiting on the original author for a response label Nov 7, 2024
@tyreseluo tyreseluo self-assigned this Nov 8, 2024
@kevinaboos
Copy link
Member

is this actually blocked on Makepad? if so, what feature of Makepad is missing? I can't think of anything in Makepad that is preventing us from implementing a basic search bar.

@tyreseluo
Copy link
Contributor Author

oh, ok,i guess I'm lagging. i will do again.

@kevinaboos
Copy link
Member

no worries, I didn't mean to imply that you were lagging. I was just curious if we actually needed more features from Makepad or not.

@tyreseluo tyreseluo removed the blocked-on-makepad Blocked on a Makepad bug or missing Makepad feature label Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-on-author This issue is waiting on the original author for a response
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement search bar at the top of the rooms list
3 participants